Derby
  1. Derby
  2. DERBY-3408

Wrong message when using SHOW in ij.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.5.1.1
    • Component/s: SQL
    • Labels:
      None

      Description

      When running the show command using ij, it gives out a wrong message.

      eg.:
      ij> show schema;
      ERROR 42X01: Syntax error: Encountered "show" at line 2, column 1.

      The message should be something like:
      Syntax error: Encountered "schema" at line 2,column 1:no such thing as "schema"

      1. afterCodeReview.txt
        5 kB
        Bryan Pendleton
      2. includeSimpleTest.diff
        5 kB
        Bryan Pendleton
      3. suggestHelp.diff
        2 kB
        Bryan Pendleton
      4. suggestServerDocsToo.diff
        2 kB
        Bryan Pendleton
      5. withUpdatedRegressionTestOutput.diff
        218 kB
        Bryan Pendleton

        Activity

        Hide
        John H. Embretsen added a comment -

        Although the current error message is not necessarily wrong, I certainly agree that it is not helpful at all, and should be improved.

        Show
        John H. Embretsen added a comment - Although the current error message is not necessarily wrong, I certainly agree that it is not helpful at all, and should be improved.
        Hide
        Bryan Pendleton added a comment -

        One simple approach would be for IJ to notice when message 42X01
        (the general all-purpose 'syntax error' message) was issued, and to
        follow it up with a simple suggestion that the user might try using the
        'help' command for general information about command syntax.

        Attached is a simple patch to do this.

        The output from IJ would then look like:

        ij> show;
        ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1.
        Issue the 'help' command for general information on IJ command syntax.
        ij> show schema;
        ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1.
        Issue the 'help' command for general information on IJ command syntax.

        This may impact some number of regression tests; I haven't
        investigated to see what the scope of the impact would be, just
        wanted to see what people thought of this approach.

        Show
        Bryan Pendleton added a comment - One simple approach would be for IJ to notice when message 42X01 (the general all-purpose 'syntax error' message) was issued, and to follow it up with a simple suggestion that the user might try using the 'help' command for general information about command syntax. Attached is a simple patch to do this. The output from IJ would then look like: ij> show; ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1. Issue the 'help' command for general information on IJ command syntax. ij> show schema; ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1. Issue the 'help' command for general information on IJ command syntax. This may impact some number of regression tests; I haven't investigated to see what the scope of the impact would be, just wanted to see what people thought of this approach.
        Hide
        Knut Anders Hatlen added a comment -

        This looks like a good improvement to me. The SQL state 42X01 doesn't necessarily mean that the user issued one of the IJ-specific commands, like for instance:

        ij> select * from t where x = 4 sort by z;
        ERROR 42X01: Syntax error: Encountered "sort" at line 1, column 29.
        Issue the 'help' command for general information on IJ command syntax.

        Perhaps we should append "or consult the reference manual for the supported SQL syntax" or something to cover what the help command is silent about?

        Show
        Knut Anders Hatlen added a comment - This looks like a good improvement to me. The SQL state 42X01 doesn't necessarily mean that the user issued one of the IJ-specific commands, like for instance: ij> select * from t where x = 4 sort by z; ERROR 42X01: Syntax error: Encountered "sort" at line 1, column 29. Issue the 'help' command for general information on IJ command syntax. Perhaps we should append "or consult the reference manual for the supported SQL syntax" or something to cover what the help command is silent about?
        Hide
        Jazarine Jamal added a comment -

        Very goood improvement indeed!

        Knut's comment is important too.

        We could have an another option of including all the errors falling under the category: message 42X01 (the general all-purpose 'syntax error' message) in the help command.

        What do everyone think of this?
        Will it increase the footprint size considerably?

        Show
        Jazarine Jamal added a comment - Very goood improvement indeed! Knut's comment is important too. We could have an another option of including all the errors falling under the category: message 42X01 (the general all-purpose 'syntax error' message) in the help command. What do everyone think of this? Will it increase the footprint size considerably?
        Hide
        Knut Anders Hatlen added a comment -

        I think 42X01 can be raised for almost any SQL statement if you mistype something, so then we'd basically have to add all the supported SQL syntax to the help text. IJ is meant to work with other JDBC drivers too, so it's probably best not to assume too much about what syntax the underlying SQL engine supports.

        Show
        Knut Anders Hatlen added a comment - I think 42X01 can be raised for almost any SQL statement if you mistype something, so then we'd basically have to add all the supported SQL syntax to the help text. IJ is meant to work with other JDBC drivers too, so it's probably best not to assume too much about what syntax the underlying SQL engine supports.
        Hide
        Bryan Pendleton added a comment -

        Thanks for the feedback! I like the idea of broadening the suggestion message
        to also suggest referring to the reference docs for SQL syntax. I agree that putting
        the complete SQL syntax in IJ's help would be problematic; in addition to the
        issues mentioned, the IJ client can be used with a range of different versions
        of the Derby server, each with its own particular version-specific dialect of
        supported SQL.

        Show
        Bryan Pendleton added a comment - Thanks for the feedback! I like the idea of broadening the suggestion message to also suggest referring to the reference docs for SQL syntax. I agree that putting the complete SQL syntax in IJ's help would be problematic; in addition to the issues mentioned, the IJ client can be used with a range of different versions of the Derby server, each with its own particular version-specific dialect of supported SQL.
        Hide
        Bryan Pendleton added a comment -

        Attached is a revised patch with an expanded suggestion message. The new output is as follows:

        -bash-2.05b$ java org.apache.derby.tools.ij
        ij version 10.5
        ij> connect 'jdbc:derby:brydb;create=true';
        ij> show;
        ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1.
        Issue the 'help' command for general information on IJ command syntax. Any unrecognized commands are treated as potential SQL commands and executed directly. Consult your DBMS server reference documentation for details of the SQL syntax supported by your server.

        Show
        Bryan Pendleton added a comment - Attached is a revised patch with an expanded suggestion message. The new output is as follows: -bash-2.05b$ java org.apache.derby.tools.ij ij version 10.5 ij> connect 'jdbc:derby:brydb;create=true'; ij> show; ERROR 42X01: Syntax error: Encountered "show" at line 1, column 1. Issue the 'help' command for general information on IJ command syntax. Any unrecognized commands are treated as potential SQL commands and executed directly. Consult your DBMS server reference documentation for details of the SQL syntax supported by your server.
        Hide
        Bryan Pendleton added a comment -

        I updated the patch proposal to include a simple
        regression test as part of ij2.sql. Please let me
        know of any additional comments regarding the patch.

        Show
        Bryan Pendleton added a comment - I updated the patch proposal to include a simple regression test as part of ij2.sql. Please let me know of any additional comments regarding the patch.
        Hide
        Knut Anders Hatlen added a comment -

        Thanks, Bryan! I think the approach looks very good. Some small comments:

        • The indentation looks wrong in utilMain.java. Probably your editor is configured with tab size 8 instead of 4.
        • In utilMain.handleSQLException() the code that sets syntaxErrorOccurred is placed between a comment and the code that is described by the comment. Perhaps it would be clearer if the new code was placed above the comment?
        • Some of the other tools messages have line breaks so that they look nice in a terminal. Should we do the same for IJ_SuggestHelp?
        Show
        Knut Anders Hatlen added a comment - Thanks, Bryan! I think the approach looks very good. Some small comments: The indentation looks wrong in utilMain.java. Probably your editor is configured with tab size 8 instead of 4. In utilMain.handleSQLException() the code that sets syntaxErrorOccurred is placed between a comment and the code that is described by the comment. Perhaps it would be clearer if the new code was placed above the comment? Some of the other tools messages have line breaks so that they look nice in a terminal. Should we do the same for IJ_SuggestHelp?
        Hide
        Bryan Pendleton added a comment -

        Thanks for the comments Knut, those are all very useful suggestions.

        I tried placing line breaks in the message at the end of each
        sentence. Depending on your terminal width the lines will
        still wrap, but those seemed like reasonable places to break the lines.

        Please have a look at 'afterCodeReview.txt' and tell me what you think.

        Show
        Bryan Pendleton added a comment - Thanks for the comments Knut, those are all very useful suggestions. I tried placing line breaks in the message at the end of each sentence. Depending on your terminal width the lines will still wrap, but those seemed like reasonable places to break the lines. Please have a look at 'afterCodeReview.txt' and tell me what you think.
        Hide
        Knut Anders Hatlen added a comment -

        Thanks Bryan, it looks much better now. +1 to commit.

        Show
        Knut Anders Hatlen added a comment - Thanks Bryan, it looks much better now. +1 to commit.
        Hide
        Bryan Pendleton added a comment -

        I've run a complete set of regression tests, and, as I suspected, I'm going to have
        to update a fair number of test output files.

        Before I go propose a big patch to do this, I'd like to respond to any other
        feedback regarding the message itself. So if anybody else has any opinions
        on the new message, please let me know.

        Show
        Bryan Pendleton added a comment - I've run a complete set of regression tests, and, as I suspected, I'm going to have to update a fair number of test output files. Before I go propose a big patch to do this, I'd like to respond to any other feedback regarding the message itself. So if anybody else has any opinions on the new message, please let me know.
        Hide
        Bryan Pendleton added a comment -

        Attached 'withUpdatedRegressionTestOutput.diff' patch proposal includes
        the same code changes as in the previous patch ('afterCodeReview.txt'),
        but also includes changes to all the various regression test output files
        to reflect the new IJ output.

        The regression test output file changes are simply to add the new text
        whenever a 42X01message is emitted.

        The regression tests now pass successfully with this patch, review would
        be appreciated.

        Show
        Bryan Pendleton added a comment - Attached 'withUpdatedRegressionTestOutput.diff' patch proposal includes the same code changes as in the previous patch ('afterCodeReview.txt'), but also includes changes to all the various regression test output files to reflect the new IJ output. The regression test output file changes are simply to add the new text whenever a 42X01message is emitted. The regression tests now pass successfully with this patch, review would be appreciated.
        Hide
        Bryan Pendleton added a comment -

        Committed to the trunk as revision 694954.

        Show
        Bryan Pendleton added a comment - Committed to the trunk as revision 694954.

          People

          • Assignee:
            Bryan Pendleton
            Reporter:
            Jazarine Jamal
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development