[CALCITE-3152] Unify throws in sql parser

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.20.0
• Fix Version/s: 1.21.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Danny Chen
• Reporter: Danny Chen
• Created: 28/Jun/19 02:13
• Updated: 10/Sep/19 21:37
• Resolved: 02/Jul/19 12:19

Description

Copied from DEV mail,

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 position info, which is not that user friendly when the sql text is huge (there are 10 occurance for 3 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.

[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

Issue Links

• links to: GitHub Pull Request #1284

Sub-Tasks

• Add test for invalid literal of sql parser: Closed