You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Piotr Bojko (JIRA)" <ji...@apache.org> on 2018/10/05 13:02:00 UTC
[jira] [Commented] (CALCITE-2613) NPE when Avatica used as JDBC
transport for Calcite and dynamic param ? used
[ https://issues.apache.org/jira/browse/CALCITE-2613?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16639796#comment-16639796 ]
Piotr Bojko commented on CALCITE-2613:
--------------------------------------
[Branch with a test case|https://github.com/ptrbojko/calcite/tree/bug/CALCITE-2613]
> NPE when Avatica used as JDBC transport for Calcite and dynamic param ? used
> ------------------------------------------------------------------------------
>
> Key: CALCITE-2613
> URL: https://issues.apache.org/jira/browse/CALCITE-2613
> Project: Calcite
> Issue Type: Bug
> Components: avatica, core
> Affects Versions: 1.17.0, avatica-1.12.0, 1.18.0
> Reporter: Piotr Bojko
> Priority: Major
>
> This is a follow from dev@apache forum discussion. When avatica is wrapping through JdbcMeta a calcite connection and prepared statement contains dynamic parameters, then AvaticaParameters.toProto throws NPE (underlying Builder of Common.AvaticaParameter to be exact).
> FYI [~elserj]
> I will attach a test case to this issue in a sec.
> {quote}
> 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@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.bojko@gmail.com <pt...@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 <el...@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.bojko@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 <francischuang@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.bojko@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
> >>
> >
> {quote}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)