OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Avatica or Calcite or mine fault?


re: nulls on className, that's how Protobuf itself works. You can't set null for an attribute.

What is the circumstance where you would have a null className on an AvaticaParameter?

We could proactively check in the constructor to AvaticaParameter that you don't provide null arguments, but that could be argued in either direction (e.g. you should not provide null arguments in the first place).

On 10/2/18 5:11 PM, ptr.bojko@xxxxxxxxx wrote:
Hello,

It seems that my case consists of two bugs:

First one is similar to what I've already patched twice - Calcite pushes
too much to underlying jdbc schema, in this case "?" operator. Resulting
subquery onto jdbc schema ends with error about it (? not resolved). This
one belongs to Calcite.

The second one is located
at org.apache.calcite.avatica.AvaticaParameter.toProto method.
AvaticaParameter can be a live nullable className but
Common.AvaticaParameter does not allow to set nulls on className. toProto
method - please verify me - should be more carefull about it and not set
className on a builder when it is null. This one belongs to Avatica.

If anyone has time to check me it would be great. I will log then each bug
to appropriate apache project in Jira. I should patch first one fairly easy.

Regards,
Piotr

On Mon, Oct 1, 2018 at 1:50 PM Francis Chuang <francischuang@xxxxxxxxxx>
wrote:

Hey Piotr,

Thanks for reporting this! I am not familiar with Avatica's internals,
so can't recommend how this can be fixed. However, I would suggest
writing a test case to reproduce the problem in the meantime.

Francis

On 1/10/2018 8:59 PM, ptr.bojko@xxxxxxxxx wrote:
Hello fellow calcite dev team :)

I have discovered the case with NPE when trying to use parameters on
prepared statement:
java.lang.NullPointerException
at

org.apache.calcite.avatica.proto.Common$AvaticaParameter$Builder.setClassName(Common.java:9040)
at

org.apache.calcite.avatica.AvaticaParameter.toProto(AvaticaParameter.java:64)
at org.apache.calcite.avatica.Meta$Signature.toProto(Meta.java:835)
at
org.apache.calcite.avatica.Meta$StatementHandle.toProto(Meta.java:1236)
at

org.apache.calcite.avatica.remote.Service$PrepareResponse.serialize(Service.java:1310)
at

org.apache.calcite.avatica.remote.Service$PrepareResponse.serialize(Service.java:1275)
at

org.apache.calcite.avatica.remote.ProtobufTranslationImpl.serializeResponse(ProtobufTranslationImpl.java:348)
at

org.apache.calcite.avatica.remote.ProtobufHandler.encode(ProtobufHandler.java:57)
at

org.apache.calcite.avatica.remote.ProtobufHandler.encode(ProtobufHandler.java:31)
at

org.apache.calcite.avatica.remote.AbstractHandler.apply(AbstractHandler.java:95)
at

org.apache.calcite.avatica.remote.ProtobufHandler.apply(ProtobufHandler.java:46)


It looks like CalcitePrepareImpl class does have one method implemented:
     private static String getClassName(RelDataType type) {
          return null;
      }
Or Common$AvaticaParameter$Builder.setClassName is too restrictive? Or
maybe AvaticaParameter.toProto() should not feed the builder with
nullable
className?

Please advice, so I could help patch this.