Derby
  1. Derby
  2. DERBY-1749

Implement Bracketed SQL comments (/*...*/ comments)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.1.3.1, 10.2.1.6, 10.2.2.0, 10.3.1.4
    • Fix Version/s: 10.4.1.3
    • Component/s: SQL
    • Labels:
      None

      Description

      Hibernate use multiline sql statements to show the internal building of the hql statements. You can disable this feature, but it would be nice if you can implements bracketed SQL comments in Derby.

      Thanks.

      1. Derby-1749-4.diff
        7 kB
        Dag H. Wanvik
      2. releaseNote.html
        4 kB
        Thomas Nielsen
      3. d1749-rrefsqlj28468.dita.diff
        1 kB
        Thomas Nielsen
      4. d1749-rrefsqlj28468.html.diff-2
        0.6 kB
        Thomas Nielsen
      5. d1749-ref-single.html.diff-2
        2 kB
        Thomas Nielsen
      6. Derby-1749-3.diff
        5 kB
        James F. Adams
      7. d1749-rrefsqlj28468.html.diff
        0.5 kB
        Thomas Nielsen
      8. d1749-ref-single.html.diff
        2 kB
        Thomas Nielsen
      9. Derby-1749-2.txt
        1.0 kB
        James F. Adams
      10. DERBY-1749.diff
        0.6 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Bernt M. Johnsen added a comment -

          This is Optional Feature T352 (http://wiki.apache.org/db-derby/SQLvsDerbyFeatures).

          Implementing this might also ease the use of optimizer overrides from Java
          code. E.g. the following SQL

          SELECT * FROM t1 – DERBY-PROPERTIES index = t1_c1
          FOR UPDATE OF c2, c1

          Will in Java look like (newline is needed to terminate the "--"
          comment):

          stmt.executeQuery("SELECT * FROM t1 – DERBY-PROPERTIES index = t1_c1 \nFOR UPDATE OF c2, c1");

          while with T352 implemented, could be something like

          stmt.executeQuery("SELECT * FROM t1 /* DERBY-PROPERTIES index = t1_c1 */ FOR UPDATE OF c2, c1");

          which I think is a bit more readable.

          Show
          Bernt M. Johnsen added a comment - This is Optional Feature T352 ( http://wiki.apache.org/db-derby/SQLvsDerbyFeatures ). Implementing this might also ease the use of optimizer overrides from Java code. E.g. the following SQL SELECT * FROM t1 – DERBY-PROPERTIES index = t1_c1 FOR UPDATE OF c2, c1 Will in Java look like (newline is needed to terminate the "--" comment): stmt.executeQuery("SELECT * FROM t1 – DERBY-PROPERTIES index = t1_c1 \nFOR UPDATE OF c2, c1"); while with T352 implemented, could be something like stmt.executeQuery("SELECT * FROM t1 /* DERBY-PROPERTIES index = t1_c1 */ FOR UPDATE OF c2, c1"); which I think is a bit more readable.
          Hide
          Bernt M. Johnsen added a comment -

          This is of course Optional Feature T351. Sorry for the typo.

          Show
          Bernt M. Johnsen added a comment - This is of course Optional Feature T351. Sorry for the typo.
          Hide
          Thomas Nielsen added a comment -

          Workaround details missing from orginal report:

          Change the "sql_comments" flag in the Hibernate configfile to disable comments in the generated sql.

          Show
          Thomas Nielsen added a comment - Workaround details missing from orginal report: Change the "sql_comments" flag in the Hibernate configfile to disable comments in the generated sql.
          Hide
          Thomas Nielsen added a comment -

          Flagging most recent released versions as affected as well.

          Show
          Thomas Nielsen added a comment - Flagging most recent released versions as affected as well.
          Hide
          Dag H. Wanvik added a comment -

          Here is a patch for sqlgrammar.jj which implements bracketed comments

          Not a complete patch; no tests added yet, running regression
          tests now.

          Show
          Dag H. Wanvik added a comment - Here is a patch for sqlgrammar.jj which implements bracketed comments Not a complete patch; no tests added yet, running regression tests now.
          Hide
          Dag H. Wanvik added a comment -

          The patch is not sufficient: the standard demands that bracketed comments
          can nest, cf. SQL 2003 section 5.2 <token> and <separator>, syntax rule 10):

          "Within a <bracketed comment contents>, any <solidus> immediately followed by
          an <asterisk> without any intervening <separator> shall be considered to be the
          <bracketed comment introducer> of a <separator> that is a <bracketed comment>. "

          Show
          Dag H. Wanvik added a comment - The patch is not sufficient: the standard demands that bracketed comments can nest, cf. SQL 2003 section 5.2 <token> and <separator>, syntax rule 10): "Within a <bracketed comment contents>, any <solidus> immediately followed by an <asterisk> without any intervening <separator> shall be considered to be the <bracketed comment introducer> of a <separator> that is a <bracketed comment>. "
          Hide
          Dag H. Wanvik added a comment -

          Unassiging myself for now.

          Show
          Dag H. Wanvik added a comment - Unassiging myself for now.
          Hide
          James F. Adams added a comment -

          Derby-1749-2 is an updated patch to sqlgrammar.jj which implements nested bracketed comments.

          Not a complete patch; no tests added yet.

          Show
          James F. Adams added a comment - Derby-1749-2 is an updated patch to sqlgrammar.jj which implements nested bracketed comments. Not a complete patch; no tests added yet.
          Hide
          Thomas Nielsen added a comment -

          Attaching proposal for updated comments text for the Reference Guide under "Capitalization and special characters".

          Show
          Thomas Nielsen added a comment - Attaching proposal for updated comments text for the Reference Guide under "Capitalization and special characters".
          Hide
          Dag H. Wanvik added a comment -

          Patch looks good, James, thanks! Do you plan to add tests also?

          Thomas, thanks for the doc updates.
          The description should mention that bracketed comments nest.
          Could you make the changes to the .dita files also?

          Show
          Dag H. Wanvik added a comment - Patch looks good, James, thanks! Do you plan to add tests also? Thomas, thanks for the doc updates. The description should mention that bracketed comments nest. Could you make the changes to the .dita files also?
          Hide
          James F. Adams added a comment -

          Attached new patch (Derby-1749-3.diff) that adds some simple bracketed comment tests.

          Show
          James F. Adams added a comment - Attached new patch (Derby-1749-3.diff) that adds some simple bracketed comment tests.
          Hide
          Thomas Nielsen added a comment -

          Updated dita source and html attached. This includes a comment on nested bracketed comments. Thanks for spotting that Dag.

          Show
          Thomas Nielsen added a comment - Updated dita source and html attached. This includes a comment on nested bracketed comments. Thanks for spotting that Dag.
          Hide
          Dag H. Wanvik added a comment -

          Thanks for making tests, too, James!
          The tests look good and passed in my sandbox.

          I will add some with a mix of eol("---") and bracketed comments for
          good measure.

          And Thomas, thanks for the doc patch, I generated the docs
          with it and it looks good.

          I will probably substitute "asterisk" for "star" to keep
          in line with typography language.

          I will commit both when my regression tests are done.

          Show
          Dag H. Wanvik added a comment - Thanks for making tests, too, James! The tests look good and passed in my sandbox. I will add some with a mix of eol("---") and bracketed comments for good measure. And Thomas, thanks for the doc patch, I generated the docs with it and it looks good. I will probably substitute "asterisk" for "star" to keep in line with typography language. I will commit both when my regression tests are done.
          Hide
          Dag H. Wanvik added a comment -

          Thomas, would you be willing to write and attach the release notes for this feature also?
          Template can be found here:

          ../tools/release/templates/releaseNote.html

          Show
          Dag H. Wanvik added a comment - Thomas, would you be willing to write and attach the release notes for this feature also? Template can be found here: ../tools/release/templates/releaseNote.html
          Hide
          Thomas Nielsen added a comment -

          +1 for the asterisk change Dag.
          Sure, I'll do a releaseNote for this issue .

          Show
          Thomas Nielsen added a comment - +1 for the asterisk change Dag. Sure, I'll do a releaseNote for this issue .
          Hide
          Thomas Nielsen added a comment -

          Attaching releaseNote.html

          Show
          Thomas Nielsen added a comment - Attaching releaseNote.html
          Hide
          Daniel John Debrunner added a comment -

          Dag, could you explain why a release note is needed. I thought release notes were for changes that might affect existing working applications, I didn't think new features had release notes?

          Show
          Daniel John Debrunner added a comment - Dag, could you explain why a release note is needed. I thought release notes were for changes that might affect existing working applications, I didn't think new features had release notes?
          Hide
          Dag H. Wanvik added a comment -
          Show
          Dag H. Wanvik added a comment - The releaseNotes.html is probably redundant here, see discussion in http://www.nabble.com/-jira--Created%3A-%28DERBY-1749%29-Implement-Bracketed-SQL-comments-%28-*...*--comments%29-t2151720i20.html
          Hide
          Dag H. Wanvik added a comment -

          Same as Derby-1749-3.diff, but adds a few new test cases.

          Show
          Dag H. Wanvik added a comment - Same as Derby-1749-3.diff, but adds a few new test cases.
          Hide
          Dag H. Wanvik added a comment -

          Committed Derby-1749-4.diff as svn 595662.

          Thanks for the good work, James!

          Show
          Dag H. Wanvik added a comment - Committed Derby-1749-4.diff as svn 595662. Thanks for the good work, James!
          Hide
          Dag H. Wanvik added a comment -

          Assigned this to you James, since you did most of the work.

          Show
          Dag H. Wanvik added a comment - Assigned this to you James, since you did most of the work.
          Hide
          Dag H. Wanvik added a comment -

          Committed patch d1749-rrefsqlj28468.dita.diff to the documentation trunk
          as svn 595671. Thanks Thomas!

          Removing release note needed flag as per the discussion we had, see previous comments.

          Resolving.

          Show
          Dag H. Wanvik added a comment - Committed patch d1749-rrefsqlj28468.dita.diff to the documentation trunk as svn 595671. Thanks Thomas! Removing release note needed flag as per the discussion we had, see previous comments. Resolving.
          Hide
          Dag H. Wanvik added a comment -

          Did not hear back from reporter, so closing.

          Show
          Dag H. Wanvik added a comment - Did not hear back from reporter, so closing.

            People

            • Assignee:
              James F. Adams
              Reporter:
              Conny Kreyßel
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development