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

Unify throws in sql parser

    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

        Issue Links

          There are no Sub-Tasks for this issue.

          Activity

            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