osdir.com

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

Re: Avatica or Calcite or mine fault?


I've logged CALCITE-2609 for calcite pushing ? to underlygin jdbc subschema
causing error on sub query to external jdbc source. I've attached the fix
but with a question on validity of on already existing unit test. Please
help me here to get further :)

As Josh suggested I've logged CALCiTE-2613 for Avatica vs Calcite managing
of AvaticaParameters causing NPE - test case is attached.

Regards,
P



On Thu, Oct 4, 2018 at 5:06 PM Josh Elser <elserj@xxxxxxxxxx> wrote:

> An AvaticaParameter with a null classname sounds wrong to me. We should
> know the type (per the schema) for some variable, regardless of whether
> or not it is nullable.
>
> If you can put together a unit test showing what you're seeing in
> Avatica, that would go an extremely long way. It may also just be
> something Calcite is doing which should be corrected (so, a unit test
> there would be a starting point, too).
>
> Feel free to ping me on Jira. Thanks Piotr!
>
> On 10/4/18 9:08 AM, ptr.bojko@xxxxxxxxx wrote:
> > 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