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 <els...@apache.org> 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.bo...@gmail.com 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.bo...@gmail.com <ptr.bo...@gmail.com> > > 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 <els...@apache.org> 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.bo...@gmail.com 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 < > francischu...@apache.org > >>>> > >>>> 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.bo...@gmail.com 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
