Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-3152

Unify throws in sql parser

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.20.0
    • Fix Version/s: 1.21.0
    • Component/s: core

      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

        Attachments

          Activity

            People

            • Assignee:
              danny0405 Danny Chen
              Reporter:
              danny0405 Danny Chen

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 50m
                50m

                  Issue deployment