-
Couldn't load subscription status.
- Fork 0
Allow to set up the default null ordering #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 introduces a configurable default null ordering for SQL queries to accommodate different database behaviors. Instead of hardcoded PostgreSQL-style null ordering, users can now specify their preferred default null ordering strategy.
- Adds a new
default_null_orderingconfiguration option with four possible values:asc_reverse,desc_reverse,nulls_first, andnulls_last - Updates the SQL parser to use the configured null ordering when explicit
NULLS FIRST/LASTclauses are not specified - Maintains PostgreSQL compatibility by defaulting to
asc_reversebehavior
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| datafusion/common/src/config.rs | Adds the new default_null_ordering configuration option |
| datafusion/sql/src/planner.rs | Defines the NullOrdering enum and implements string parsing logic |
| datafusion/sql/src/expr/order_by.rs | Updates ORDER BY expression handling to use configurable null ordering |
| datafusion/sql/src/statement.rs | Updates window function ordering to use configurable null ordering |
| datafusion/core/src/execution/session_state.rs | Integrates the configuration into session state |
| datafusion/sql/tests/sql_integration.rs | Updates test cases to include the new configuration parameter |
| datafusion/sqllogictest/test_files/order.slt | Adds comprehensive test coverage for all null ordering configurations |
| datafusion/sqllogictest/test_files/information_schema.slt | Updates information schema output to include the new configuration |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Let's ignore the MSRV check. DataFusion 46.0.0 is too old. We should move to the latest version as soon as possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NULLS FIRST and NULLS LAST options can be used to determine whether nulls appear before or after non-null values in the sort ordering. By default, null values sort as if larger than any non-null value; that is, NULLS FIRST is the default for DESC order, and NULLS LAST otherwise. -- Postgres doc
TLDR
desc_reverse flips the default NULL placement: ASC gives NULLS FIRST, DESC gives NULLS LAST; opposite of standard.
asc_reverse flips the default NULL placement: ASC gives NULLS LAST, DESC gives NULLS FIRST; matches the standard SQL behavior.
|
Thanks @goldmedal |
… default null ordering (apache#16963) * Allow to set up the default null ordering (#3) * add default_null_ordering config * add test for different config * Update datafusion/sql/src/planner.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * update doc * fix sqllogictest --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * rename config and fix test * update doc * fix default * fix doc * fix sqllogictests * address comments --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Which issue does this PR close?
Rationale for this change
To fit different requirements of default null ordering, this PR introduces a new config to set up the default null ordering when parsing SQL.
There are 4 cases for the null ordering:
Postgres is
AscReverse. So we set it by default to follow the PostgreSQL behavior.DuckDB is
NullsLast. If users want to follow it, they can configure it toNullsLasteasily.What changes are included in this PR?
add new config.
Are these changes tested?
add sqllogictest
Are there any user-facing changes?