Thanks for the suggestion Stamatis, but that didn't work for me. It caused compilation errors in SqlParserImpl and I couldn't see a way to resolve them.
Thanks, Gelbana On Sun, Mar 31, 2019 at 6:01 PM Stamatis Zampetakis <zabe...@gmail.com> wrote: > I am not sure why the update breaks so many tests neither if there is a > problem with the LOOKAHEAD but regarding CALCITE-2844, I would be inclined > to modify lines around [3] to make it work. > > In particular, I would try to make <TABLE> with parentheses optional (just > in case you didn't try this so far). > > Best, > Stamatis > > [3] > > https://github.com/apache/calcite/blob/81fa5314e94e86b6cf8df244b03f9d57c884f54d/core/src/main/codegen/templates/Parser.jj#L1884 > > Στις Κυρ, 31 Μαρ 2019 στις 5:16 μ.μ., ο/η Muhammad Gelbana < > m.gelb...@gmail.com> έγραψε: > > > I was trying to support selecting from table functions[1]. I tried > > extending TableRef2[2] (Production ?) to support table functions by > adding > > > > > LOOKAHEAD(3) > > > > > tableRef = TableFunctionCall(getPos())) > > > > > | > > > > > before > > > > > LOOKAHEAD(2) > > > tableRef = CompoundIdentifier() > > > > > > > but it broke other tests. I tried putting my modification at the end of > the > > choices while increasing the CompoundIdentifier() lookahead to 3 to avoid > > that choice when it faces the left bracket, but it didn't work too. I > tried > > setting aggresively high lookahead values such as 50, and it didn't work > > too. I won't be surprised if I'm doing anything wrong as I'm not > accustomed > > to working with grammar files anyway. > > > > The only thing I'm considering now is to create a new production (I'm not > > sure if I'm using this word correctly) such as TableRef3 and have that > > going down the common path between TableFunctionCall() and > > CompoundIdentifier() because TableFunctionCall() eventually attempts to > > cosnume a CompoundIdentifier(). This way I won't have to bother about > > tuning lookaheads I suppose. > > > > I can create a branch of what I've accomplished so far if you wish. > > > > [1] https://issues.apache.org/jira/browse/CALCITE-2844 > > [2] > > > > > https://github.com/apache/calcite/blob/master/core/src/main/codegen/templates/Parser.jj#L1811 > > > > Thanks, > > Gelbana > > > > > > On Sun, Mar 31, 2019 at 4:15 PM Hongze Zhang <notify...@126.com> wrote: > > > > > Just out of my curiosity, could you please share your case about > > > "LOOKAHEAD doest not work as expected"? Does changing to JavaCC 5.0 > > > actually fixes the problem? > > > > > > Thanks, > > > Hongze > > > > > > > > > > On Mar 31, 2019, at 19:17, Muhammad Gelbana <m.gelb...@gmail.com> > > wrote: > > > > > > > > I'm facing trouble with supporting selecting from table function for > > > Babel > > > > parser and I beleive that LOOKAHEAD isn't working as expected too. > > > > I thought it might actually be a bug so I checked out the master > branch > > > and > > > > updated the JavaCC maven plugin version to 2.6 (it's currently 2.4), > > but > > > > that failed *142* test cases and errored *9*. > > > > > > > > The plugin v2.4 imports the JavaCC library v4 > > > > The plugin v2.6 imports the JavaCC library v5 > > > > > > > > Unfortunately the release notes for the JavaCC library are broken and > > I'm > > > > not aware of another source for the release notes for that project. > > > > Should I open a Jira to upgrade that plugin version ? > > > > > > > > Thanks, > > > > Gelbana > > > > > > > > > > > > On Thu, Mar 28, 2019 at 4:18 AM Rui Li <lirui.fu...@gmail.com> > wrote: > > > > > > > >> Thanks Hongze, that's good to know. > > > >> > > > >> On Thu, Mar 28, 2019 at 8:43 AM Hongze Zhang <notify...@126.com> > > wrote: > > > >> > > > >>>> Besides, if I enable forceLaCheck, JavaCC suggests to use a > > lookahead > > > >> of > > > >>> 3 > > > >>>> or more. I guess we'd better get rid of these warnings if we want > to > > > >>> stick > > > >>>> to lookahead(2). > > > >>> > > > >>> That makes sense. Actually we had a discussion[1] on moving to > > > >>> "LOOKAHEAD=1", and seems we are close to finish it. By doing this > we > > > have > > > >>> extra benefits that we don't need to turn forceLaCheck on and > JavaCC > > > >> should > > > >>> give suggestions during maven build. > > > >>> > > > >>> Hongze > > > >>> > > > >>> > > > >>> [1] https://issues.apache.org/jira/browse/CALCITE-2847 > > > >>> > > > >>>> On Mar 27, 2019, at 10:40, Rui Li <lirui.fu...@gmail.com> wrote: > > > >>>> > > > >>>> Thanks Hongze for looking into the issue! Are you suggesting this > is > > > >> more > > > >>>> likely to be a JavaCC bug? > > > >>>> I filed a ticket anyway in case we want to further track it: > > > >>>> https://issues.apache.org/jira/browse/CALCITE-2957 > > > >>>> Besides, if I enable forceLaCheck, JavaCC suggests to use a > > lookahead > > > >> of > > > >>> 3 > > > >>>> or more. I guess we'd better get rid of these warnings if we want > to > > > >>> stick > > > >>>> to lookahead(2). > > > >>>> > > > >>>> On Wed, Mar 27, 2019 at 8:54 AM Hongze Zhang <notify...@126.com> > > > >> wrote: > > > >>>> > > > >>>>> Thanks, Yuzhao. > > > >>>>> > > > >>>>> Since the more generic problem is that the production "E()"[1] > > causes > > > >>> the > > > >>>>> parent production's looking ahead returns too early, I tried to > > find > > > a > > > >>> bad > > > >>>>> case of the same reason under current default setting LOOKAHEAD=2 > > but > > > >> it > > > >>>>> seems that under this number we didn't have a chance to meet the > > > issue > > > >>> yet. > > > >>>>> > > > >>>>> So after that I suggest to not to treat this as a Calcite's issue > > > >>>>> currently. > > > >>>>> > > > >>>>> Best, > > > >>>>> Hongze > > > >>>>> > > > >>>>> [1] > > > >>>>> > > > >>> > > > >> > > > > > > https://github.com/apache/calcite/blob/11c067f9992d9c8bc29e2326dd8b299ad1e9dbdc/core/src/main/codegen/templates/Parser.jj#L335 > > > >>>>> > > > >>>>>> On Mar 26, 2019, at 20:42, Yuzhao Chen <yuzhao....@gmail.com> > > > wrote: > > > >>>>>> > > > >>>>>> Maybe we should fire a jira if it is a bug. > > > >>>>>> > > > >>>>>> Best, > > > >>>>>> Danny Chan > > > >>>>>> 在 2019年3月26日 +0800 PM8:33，Hongze Zhang <notify...@126.com>，写道： > > > >>>>>>> Ops, correct a typo: > > > >>>>>>> > > > >>>>>>> "... after uncommenting a line ..." -> "... after commenting a > > line > > > >>>>>>> ...". > > > >>>>>>> > > > >>>>>>> Best, > > > >>>>>>> Hongze > > > >>>>>>> > > > >>>>>>> ------ Original Message ------ > > > >>>>>>> From: "Hongze Zhang" <notify...@126.com> > > > >>>>>>> To: dev@calcite.apache.org > > > >>>>>>> Sent: 2019/3/26 19:28:08 > > > >>>>>>> Subject: Re: Calcite doesn't work with LOOKAHEAD(3) > > > >>>>>>> > > > >>>>>>>> Firstly, thank you very much for sharing the case, Rui! > > > >>>>>>>> > > > >>>>>>>> I have run a test with the SQL you provided and also run into > > the > > > >>> same > > > >>>>> exception (under a global LOOKAHEAD 3). After debugging the > > generated > > > >>>>> parser code, I think the problem is probably in the generated > > > >> LOOKAHEAD > > > >>>>> method SqlParserImpl#jj_3R_42(): > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>>> final private boolean jj_3R_42() { > > > >>>>>>>>> if (!jj_rescan) trace_call("SqlSelect(LOOKING AHEAD...)"); > > > >>>>>>>>> if (jj_scan_token(SELECT)) { if (!jj_rescan) > > > >>>>> trace_return("SqlSelect(LOOKAHEAD FAILED)"); return true; } > > > >>>>>>>>> if (jj_3R_190()) { if (!jj_rescan) > > > >> trace_return("SqlSelect(LOOKAHEAD > > > >>>>> FAILED)"); return true; } > > > >>>>>>>>> { if (!jj_rescan) trace_return("SqlSelect(LOOKAHEAD > > SUCCEEDED)"); > > > >>>>> return false; } > > > >>>>>>>>> } > > > >>>>>>>> > > > >>>>>>>> The LOOKAHEAD method checks only a single token <SELECT>. This > > is > > > >>>>> definitely not enough since we have already set the number to 3. > > > >>>>>>>> > > > >>>>>>>> Unfortunately I didn't find a root cause so far, but after > > > >>>>> uncommenting a line[1] in production "SqlSelect()" then > everything > > > >> goes > > > >>>>> back to normal. I'm inclined to believe JavaCC has some > unexpected > > > >>> behavior > > > >>>>> when dealing with LOOKAHEAD on a production with the shape like > > > >>>>> "SqlSelectKeywords()"[2]. > > > >>>>>>>> > > > >>>>>>>> Please feel free to log a JIRA ticket with which we can track > > > >> further > > > >>>>> information of the issue. > > > >>>>>>>> > > > >>>>>>>> Best, > > > >>>>>>>> Hongze > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> [1] > > > >>>>> > > > >>> > > > >> > > > > > > https://github.com/apache/calcite/blob/1b430721c0d9e22b2252ffcd893b42959cb7966c/core/src/main/codegen/templates/Parser.jj#L1030 > > > >>>>>>>> [2] > > > >>>>> > > > >>> > > > >> > > > > > > https://github.com/apache/calcite/blob/1b430721c0d9e22b2252ffcd893b42959cb7966c/core/src/main/codegen/templates/Parser.jj#L288 > > > >>>>>>>> > > > >>>>>>>> ------ Original Message ------ > > > >>>>>>>> From: "Rui Li" <lirui.fu...@gmail.com> > > > >>>>>>>> To: dev@calcite.apache.org > > > >>>>>>>> Sent: 2019/3/26 16:53:44 > > > >>>>>>>> Subject: Calcite doesn't work with LOOKAHEAD(3) > > > >>>>>>>> > > > >>>>>>>>> Hi, > > > >>>>>>>>> > > > >>>>>>>>> I'm trying to extend Calcite grammar to support some custom > > > >>>>> statements. And > > > >>>>>>>>> I need to increase LOOKAHEAD to 3 to resolve some ambiguity. > > But > > > >>> when > > > >>>>> I did > > > >>>>>>>>> that, the parser fails to parse queries like: > > > >>>>>>>>> * select t.key from (select key from src) t* > > > >>>>>>>>> > > > >>>>>>>>> With exception: > > > >>>>>>>>> *Caused by: > org.apache.calcite.sql.parser.impl.ParseException:* > > > >>>>>>>>> *Encountered "( select key" at line 1, column 19.* > > > >>>>>>>>> *Was expecting one of:* > > > >>>>>>>>> * <IDENTIFIER> ...* > > > >>>>>>>>> * <QUOTED_IDENTIFIER> ...* > > > >>>>>>>>> * <BACK_QUOTED_IDENTIFIER> ...* > > > >>>>>>>>> * <BRACKET_QUOTED_IDENTIFIER> ...* > > > >>>>>>>>> * <UNICODE_QUOTED_IDENTIFIER> ...* > > > >>>>>>>>> * "LATERAL" ...* > > > >>>>>>>>> * "(" "WITH" ...* > > > >>>>>>>>> *...* > > > >>>>>>>>> > > > >>>>>>>>> So I'm wondering whether there's some limitation on the > > LOOKAHEAD > > > >> we > > > >>>>> can > > > >>>>>>>>> use? > > > >>>>>>>>> > > > >>>>>>>>> -- > > > >>>>>>>>> Best regards! > > > >>>>>>>>> Rui Li > > > >>>>> > > > >>>>> > > > >>>> > > > >>>> -- > > > >>>> Best regards! > > > >>>> Rui Li > > > >>> > > > >>> > > > >> > > > >> -- > > > >> Best regards! > > > >> Rui Li > > > >> > > > > > > > > >
