Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.6.1.0
    • Component/s: None
    • Labels:
      None
    1. addSyntaxAndExample.diff
      3 kB
      Bryan Pendleton
    2. fixIntentationOfExample.diff
      3 kB
      Bryan Pendleton
    3. rrefsqlj32654.html
      5 kB
      Bryan Pendleton
    4. rrefsqlj32654.html
      5 kB
      Bryan Pendleton
    5. rrefsqlj32654.html
      5 kB
      Bryan Pendleton
    6. rrefsqlj32654.html
      5 kB
      Bryan Pendleton
    7. useBraces.diff
      3 kB
      Bryan Pendleton
    8. useBracesDifferently.diff
      3 kB
      Bryan Pendleton

      Activity

      Hide
      Bryan Pendleton added a comment -

      Committed to the docs trunk as svn revision 826126,

      Show
      Bryan Pendleton added a comment - Committed to the docs trunk as svn revision 826126,
      Hide
      Kim Haase added a comment -

      This looks just fine.

      I'm amazed you found any near-consistent model to base your syntax diagram on, but you picked a good readable one.

      Show
      Kim Haase added a comment - This looks just fine. I'm amazed you found any near-consistent model to base your syntax diagram on, but you picked a good readable one.
      Hide
      Bryan Pendleton added a comment -

      Thanks Kim and Knut for the continued review and assistance.

      I agree with Kim's latest comment: only one set of braces seems needed.

      I took a look at a couple of the other pages in the ref guide, and tried to
      structure things so that the syntax diagram is more similar to those other pages.

      Show
      Bryan Pendleton added a comment - Thanks Kim and Knut for the continued review and assistance. I agree with Kim's latest comment: only one set of braces seems needed. I took a look at a couple of the other pages in the ref guide, and tried to structure things so that the syntax diagram is more similar to those other pages.
      Hide
      Kim Haase added a comment -

      Actually – sorry I missed this before – but shouldn't it be just one set of curly braces, with the two choices (separated by the OR bar) inside?

      GROUP BY

      { column-Name [ , column-Name ]* | ROLLUP ( column-Name [ , column-Name ]* ) }
      Show
      Kim Haase added a comment - Actually – sorry I missed this before – but shouldn't it be just one set of curly braces, with the two choices (separated by the OR bar) inside? GROUP BY { column-Name [ , column-Name ]* | ROLLUP ( column-Name [ , column-Name ]* ) }
      Hide
      Knut Anders Hatlen added a comment -

      Thanks Bryan, the latest patch looks great! +1 to commit.

      Show
      Knut Anders Hatlen added a comment - Thanks Bryan, the latest patch looks great! +1 to commit.
      Hide
      Bryan Pendleton added a comment -

      Thanks Knut! I was all tripped up between braces and square brackets; I think the page looks cleaner now.

      Show
      Bryan Pendleton added a comment - Thanks Knut! I was all tripped up between braces and square brackets; I think the page looks cleaner now.
      Hide
      Knut Anders Hatlen added a comment -

      In the syntax description, it looks like some square brackets have been misplaced so that the entire list of column names has been made optional. Using

      { and }

      for grouping may also make the syntax easier to read. I suggest something like this:

      GROUP BY

      { column-Name [ , column-Name ]* }

      |

      { ROLLUP ( column-Name [ , column-Name ]* ) }

      The sentence starting with "Using the ROLLUP syntax" lacks a full stop. Perhaps it's better to say "ROLLUP keyword" instead of "ROLLUP syntax" too?

      The existing examples use upper case SQL statements. For consistency, we may want to use upper case in the new example too.

      Show
      Knut Anders Hatlen added a comment - In the syntax description, it looks like some square brackets have been misplaced so that the entire list of column names has been made optional. Using { and } for grouping may also make the syntax easier to read. I suggest something like this: GROUP BY { column-Name [ , column-Name ]* } | { ROLLUP ( column-Name [ , column-Name ]* ) } The sentence starting with "Using the ROLLUP syntax" lacks a full stop. Perhaps it's better to say "ROLLUP keyword" instead of "ROLLUP syntax" too? The existing examples use upper case SQL statements. For consistency, we may want to use upper case in the new example too.
      Hide
      Bryan Pendleton added a comment -

      Thanks Kim! I fixed the indentation of the comments in the examples section.

      Show
      Bryan Pendleton added a comment - Thanks Kim! I fixed the indentation of the comments in the examples section.
      Hide
      Kim Haase added a comment -

      This looks great, Bryan. Thanks for separating out the existing examples too.

      You might want to left-align the second line of each of the last two comments – it looks odd indented.

      Show
      Kim Haase added a comment - This looks great, Bryan. Thanks for separating out the existing examples too. You might want to left-align the second line of each of the last two comments – it looks odd indented.
      Hide
      Bryan Pendleton added a comment -

      Attached is a tiny docs patch which simply adds the new syntax and a trivial
      example to the GROUP BY page in the reference manual.

      The updated HTML page is attached for easier review.

      Show
      Bryan Pendleton added a comment - Attached is a tiny docs patch which simply adds the new syntax and a trivial example to the GROUP BY page in the reference manual. The updated HTML page is attached for easier review.

        People

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

          Dates

          • Created:
            Updated:
            Resolved:

            Development