Skip to content

Conversation

@IsmailTosunTnyl
Copy link
Contributor

GROUPING_ID is not supported with ORDER BY in the Nereids planner. Since Nereids is used mostly in 3.x and fully in 4.x, the documentation should be updated. The error message is: LOGICAL_SORT cannot contain GroupingScalarFunction expression: Grouping_Id(department, level).

Screenshot from 2025-12-06 21-13-33

Versions

  • dev
  • 4.x
  • 3.x
  • 2.1

Languages

  • Chinese
  • English

Docs Checklist

  • Checked by AI
  • Test Cases Built

Copilot AI review requested due to automatic review settings December 6, 2025 19:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the GROUPING_ID function documentation to address a limitation in the Nereids planner (versions 3.x and 4.x) where GROUPING_ID cannot be used in ORDER BY clauses. The examples are modified to use a standard column in the ORDER BY clause instead.

Key Changes:

  • Changed ORDER BY clause from GROUPING_ID(department, level) ASC to ORDER BY department desc in example queries
  • Updated expected output tables to reflect the new ordering (by department descending)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
versioned_docs/version-4.x/sql-manual/sql-functions/scalar-functions/other-functions/grouping-id.md Updated Example A to use ORDER BY department desc instead of GROUPING_ID, with corresponding output changes
versioned_docs/version-3.x/sql-manual/sql-functions/aggregate-functions/grouping-id.md Updated Example A to use ORDER BY department desc instead of GROUPING_ID, with corresponding output changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 53 to 65
| Technology | Senior | 3 |
| Sales | Senior | 1 |
| Technology | Total: Technology | 3 |
| Sales | Assistant | 2 |
| Sales | Total: Sales | 4 |
| Sales | Trainee | 1 |
| Sales | Senior | 1 |
| Marketing | Senior | 1 |
| Marketing | Trainee | 2 |
| Marketing | Assistant | 1 |
| Board of Directors | Total: Board of Directors | 2 |
| Technology | Total: Technology | 3 |
| Sales | Total: Sales | 4 |
| Marketing | Total: Marketing | 4 |
| Marketing | Trainee | 2 |
| Board of Directors | Senior | 2 |
| Board of Directors | Total: Board of Directors | 2 |
| NULL | Total: Company | 13 |
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected output may be incorrect for ORDER BY department DESC. In most SQL databases, when ordering by a column in descending order, NULL values typically appear first (as they are considered the highest value). However, this output shows NULL at the bottom. Please verify this output matches the actual query results, or consider adding a note about NULL handling in the ordering if this behavior is specific to this database system.

Copilot uses AI. Check for mistakes.
Comment on lines 53 to 65
| Technology | Senior | 3 |
| Sales | Senior | 1 |
| Technology | Total: Technology | 3 |
| Sales | Assistant | 2 |
| Sales | Total: Sales | 4 |
| Sales | Trainee | 1 |
| Sales | Senior | 1 |
| Marketing | Senior | 1 |
| Marketing | Trainee | 2 |
| Marketing | Assistant | 1 |
| Board of Directors | Total: Board of Directors | 2 |
| Technology | Total: Technology | 3 |
| Sales | Total: Sales | 4 |
| Marketing | Total: Marketing | 4 |
| Marketing | Trainee | 2 |
| Board of Directors | Senior | 2 |
| Board of Directors | Total: Board of Directors | 2 |
| NULL | Total: Company | 13 |
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected output may be incorrect for ORDER BY department DESC. In most SQL databases, when ordering by a column in descending order, NULL values typically appear first (as they are considered the highest value). However, this output shows NULL at the bottom. Please verify this output matches the actual query results, or consider adding a note about NULL handling in the ordering if this behavior is specific to this database system.

Copilot uses AI. Check for mistakes.
@morrySnow morrySnow merged commit 3b4876f into apache:master Dec 15, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants