Thanks Julian, I’ve fired a JIRA and start working on this.
Best, Danny Chan 在 2019年6月28日 +0800 AM2:44，Julian Hyde <[email protected]>，写道： > Unifying in general sounds like a good idea. I haven’t thought through the > specifics in this case. > > Note that each time we generate a parser using JavaCC — which we do in > several places in the code — it generates a new exception class. We don’t > want to use those generated classes. We would like to convert those > exceptions to non-generated exception classes, and I believe we do. > > It definitely be good if locations are converted to use SqlParserPos, i.e. > (startCol, startLine, endCol, endLine) rather than (col, line) or a position > embedded in a string. > > Julian > > > > On Jun 26, 2019, at 3:54 AM, Danny Chan <[email protected]> wrote: > > > > Now our parser has 3 kinds of throws behavior > > > > [1] Use JavaCC generateParseException > > [2] Use SqlUtil.newContextException > > [3] Use JavaCC ParseExecption directly > > > > For [1] and [2] there is a position info in the exception message, a throw > > may like: > > > From line 1, column 15 to line 1, column 26: > > > > But for 3, we only have the error message without pos info, which is not > > that user friendly when > > the sql text is huge (there are 10 occurance in our parser). > > > > So shall we unify them ? E.G. Use only 1 and 2 is enough for all the cases, > > the 2 can totally replace 3. > > > > Do you think this is necessary ? > > > > > > [1] > > https://github.com/apache/calcite/blob/69c8053cd98ec65c55fa1c3b282b076536ab758f/core/src/main/codegen/templates/Parser.jj#L4494 > > [2] > > https://github.com/apache/calcite/blob/69c8053cd98ec65c55fa1c3b282b076536ab758f/core/src/main/codegen/templates/Parser.jj#L386 > > [3] > > https://github.com/apache/calcite/blob/69c8053cd98ec65c55fa1c3b282b076536ab758f/core/src/main/codegen/templates/Parser.jj#L601 > > > > Best, > > Danny Chan >
