shinzoxD opened a new pull request, #24400:
URL: https://github.com/apache/datafusion/pull/24400

   ## Which issue does this PR close?
   
   - Part of #17498
   
   ## Rationale for this change
   
   `SET datafusion.sql_parser.default_null_ordering` accepted any string. 
Invalid values such as `nulls_mx` or an empty string succeeded and later 
silently fell back to `nulls_max` when planning `ORDER BY`.
   
   That is the same class of bug as `explain.format` in #17498: the option 
documents a fixed set of values, but the config store is a free-form string.
   
   ## What changes are included in this PR?
   
   - Move `NullOrdering` into `datafusion-common` and use it as the type of 
`SqlParserOptions::default_null_ordering`.
   - Reject unknown values at SET time, with an error that lists the valid 
options (`nulls_max`, `nulls_min`, `nulls_first`, `nulls_last`).
   - Keep `datafusion_sql::planner::NullOrdering` as a re-export so existing 
planner call sites keep compiling.
   - Leave the previous valid value in place when a SET is rejected.
   
   This is intentionally scoped to one option, following the review guidance on 
#17498 to land these as smaller PRs.
   
   ## Are these changes tested?
   
   Yes.
   
   - `cargo test -p datafusion-common default_null_ordering`
   - `cargo test -p datafusion-sqllogictest --test sqllogictests -- 
set_variable`
   - `cargo check -p datafusion --lib`
   - `cargo fmt` on the touched Rust files
   
   Coverage includes valid values, case-insensitive input, typo rejection, 
empty-string rejection, and RESET back to `nulls_max`.
   
   ## Are there any user-facing changes?
   
   Yes.
   
   - Invalid `SET datafusion.sql_parser.default_null_ordering` values now fail 
immediately instead of being stored and later treated as `nulls_max`.
   - `SqlParserOptions::default_null_ordering` is now `NullOrdering` rather 
than `String`. `NullOrdering` remains available from `datafusion_sql::planner`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to