git.net

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

Re: Avatica or Calcite or mine fault?


I've logged a JIRA for the first bug, with a fix proposal -
https://issues.apache.org/jira/browse/CALCITE-2609.

But the case with AvaticaParameter NPE still remains unclear for me - in
terms of design. Help needed :)

On Wed, Oct 3, 2018 at 4:14 PM ptr.bojko@xxxxxxxxx <ptr.bojko@xxxxxxxxx>
wrote:

> As for the nulls in AvaticaParameter class - I believe this is how calcite
> works. For example for the following query *"select * from filters where
> name = ?"* - despite the definition of filters table - Calcite will
> create org.apache.calcite.avatica.AvaticaParameter with nullable className.
>
> My case with more details is following: based on calcite-avatica/sever
> I've created a mini jdbc server (servlet) exposing Calcite connection with
> my custom schemas. I would like to expose my domain as an SQL database so
> with Calcite as an engine and domain translation and Avatica as a JDBC
> implementor the resulting implementation is very elegant and works great.
>
> However the same  query *"select * from filters where name = ?" *run
> through avatica jdbc implementation is resulting NPE
> in Common.AvaticaParameter AvaticaParameter.toProto().
>
> On Wed, Oct 3, 2018 at 12:46 AM Josh Elser <elserj@xxxxxxxxxx> wrote:
>
>> 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.
>> >>>
>> >>
>> >>
>> >
>>
>
>
> --
> Piotr Bojko
> http://about.me/ptr.bojko
>


-- 
Piotr Bojko
http://about.me/ptr.bojko