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

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.20.0
    • 1.21.0
    • 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

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            danny0405 Danny Chen
            danny0405 Danny Chen
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

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

                Slack

                  Issue deployment