minor: validate config default_null_ordering when setting it

datafusion/common/src/config.rs

@@ -311,7 +311,7 @@ config_namespace! {
         ///
         /// By default, `nulls_max` is used to follow Postgres's behavior.
         /// postgres rule: <https://www.postgresql.org/docs/current/queries-order.html>
-        pub default_null_ordering: String, default = "nulls_max".to_string()
+        pub default_null_ordering: NullOrdering, default = NullOrdering::NullsMax
 
         /// When set to true, DataFusion may remove `ORDER BY` clauses from
         /// subqueries or CTEs during SQL planning when their ordering cannot
@@ -795,6 +795,80 @@ impl Display for MapKeyDedupPolicy {
     }
 }
 
+/// Default null ordering for query results when `ORDER BY` does not specify
+/// `NULLS FIRST` or `NULLS LAST`.
+///
+/// Valid values are validated at configuration time so invalid
+/// `datafusion.sql_parser.default_null_ordering` settings are rejected
+/// immediately instead of being stored and later treated as `nulls_max`.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum NullOrdering {
+    /// Nulls appear last in ascending order and first in descending order.
+    #[default]
+    NullsMax,
+    /// Nulls appear first in ascending order and last in descending order.
+    NullsMin,
+    /// Nulls always appear first.
+    NullsFirst,
+    /// Nulls always appear last.
+    NullsLast,
+}
+
+impl NullOrdering {
+    /// Evaluates the null ordering based on the given ascending flag.
+    ///
+    /// # Returns
+    /// * `true` if nulls should appear first.
+    /// * `false` if nulls should appear last.
+    pub fn nulls_first(&self, asc: bool) -> bool {
+        match self {
+            Self::NullsMax => !asc,
+            Self::NullsMin => asc,
+            Self::NullsFirst => true,
+            Self::NullsLast => false,
+        }
+    }
+}
+
+impl FromStr for NullOrdering {
+    type Err = DataFusionError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.to_ascii_lowercase().as_str() {
+            "nulls_max" => Ok(Self::NullsMax),
+            "nulls_min" => Ok(Self::NullsMin),
+            "nulls_first" => Ok(Self::NullsFirst),
+            "nulls_last" => Ok(Self::NullsLast),
+            other => Err(DataFusionError::Configuration(format!(
+                "Invalid default null ordering: {other}. Expected one of: nulls_max, nulls_min, nulls_first, nulls_last"
+            ))),
+        }
+    }
+}
+
+impl Display for NullOrdering {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s = match self {
+            Self::NullsMax => "nulls_max",
+            Self::NullsMin => "nulls_min",
+            Self::NullsFirst => "nulls_first",
+            Self::NullsLast => "nulls_last",
+        };
+        write!(f, "{s}")
+    }
+}
+
+impl ConfigField for NullOrdering {
+    fn visit<V: Visit>(&self, v: &mut V, key: &str, description: &'static str) {
+        v.some(key, self, description)
+    }
+
+    fn set(&mut self, _: &str, value: &str) -> Result<()> {
+        *self = Self::from_str(value)?;
+        Ok(())
+    }
+}
+
 impl From<SpillCompression> for Option<CompressionType> {
     fn from(c: SpillCompression) -> Self {
         match c {
@@ -4443,6 +4517,53 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_default_null_ordering_validation() {
+        use crate::assert_contains;
+        use crate::config::{ConfigOptions, NullOrdering};
+
+        let mut config = ConfigOptions::default();
+        assert_eq!(
+            config.sql_parser.default_null_ordering,
+            NullOrdering::NullsMax
+        );
+
+        for (value, expected) in [
+            ("nulls_max", NullOrdering::NullsMax),
+            ("nulls_min", NullOrdering::NullsMin),
+            ("nulls_first", NullOrdering::NullsFirst),
+            ("nulls_last", NullOrdering::NullsLast),
+            // Values are case-insensitive, matching other enum config options.
+            ("NULLS_FIRST", NullOrdering::NullsFirst),
+        ] {
+            config
+                .set("datafusion.sql_parser.default_null_ordering", value)
+                .unwrap();
+            assert_eq!(config.sql_parser.default_null_ordering, expected);
+        }
+
+        // Invalid values, including the previous silent-fallback empty string,
+        // should error immediately at SET time.
+        for value in ["nuls_max", "", "nulls first"] {
+            let err = config
+                .set("datafusion.sql_parser.default_null_ordering", value)
+                .unwrap_err();
+            assert_contains!(
+                err.to_string(),
+                "Invalid or Unsupported Configuration: Invalid default null ordering:"
+            );
+            assert_contains!(
+                err.to_string(),
+                "Expected one of: nulls_max, nulls_min, nulls_first, nulls_last"
+            );
+            // Previous valid value remains active on error.
+            assert_eq!(
+                config.sql_parser.default_null_ordering,
+                NullOrdering::NullsFirst
+            );
+        }
+    }
+
     #[cfg(feature = "parquet")]
     #[test]
     fn set_cdc_enabled_flag() {

datafusion/core/src/execution/session_state.rs

@@ -609,10 +609,7 @@ impl SessionState {
             support_varchar_with_length: sql_parser_options.support_varchar_with_length,
             map_string_types_to_utf8view: sql_parser_options.map_string_types_to_utf8view,
             collect_spans: sql_parser_options.collect_spans,
-            default_null_ordering: sql_parser_options
-                .default_null_ordering
-                .as_str()
-                .into(),
+            default_null_ordering: sql_parser_options.default_null_ordering,
         }
     }
 

datafusion/sql/src/planner.rs

@@ -17,13 +17,14 @@
 
 //! [`SqlToRel`]: SQL Query Planner (produces [`LogicalPlan`] from SQL AST)
 use std::collections::HashMap;
-use std::str::FromStr;
 use std::sync::{Arc, Mutex};
 use std::vec;
 
 use crate::utils::make_decimal_type;
 use arrow::datatypes::*;
 use datafusion_common::TableReference;
+/// Default null ordering for sorting expressions.
+pub use datafusion_common::config::NullOrdering;
 use datafusion_common::config::SqlParserOptions;
 use datafusion_common::datatype::{DataTypeExt, FieldExt};
 use datafusion_common::error::add_possible_columns_to_diag;
@@ -159,62 +160,11 @@ impl From<&SqlParserOptions> for ParserOptions {
             enable_options_value_normalization: options
                 .enable_options_value_normalization,
             collect_spans: options.collect_spans,
-            default_null_ordering: options.default_null_ordering.as_str().into(),
+            default_null_ordering: options.default_null_ordering,
         }
     }
 }
 
-/// Represents the null ordering for sorting expressions.
-#[derive(Debug, Clone, Copy)]
-pub enum NullOrdering {
-    /// Nulls appear last in ascending order.
-    NullsMax,
-    /// Nulls appear first in descending order.
-    NullsMin,
-    /// Nulls appear first.
-    NullsFirst,
-    /// Nulls appear last.
-    NullsLast,
-}
-
-impl NullOrdering {
-    /// Evaluates the null ordering based on the given ascending flag.
-    ///
-    /// # Returns
-    /// * `true` if nulls should appear first.
-    /// * `false` if nulls should appear last.
-    pub fn nulls_first(&self, asc: bool) -> bool {
-        match self {
-            Self::NullsMax => !asc,
-            Self::NullsMin => asc,
-            Self::NullsFirst => true,
-            Self::NullsLast => false,
-        }
-    }
-}
-
-impl FromStr for NullOrdering {
-    type Err = DataFusionError;
-
-    fn from_str(s: &str) -> Result<Self> {
-        match s {
-            "nulls_max" => Ok(Self::NullsMax),
-            "nulls_min" => Ok(Self::NullsMin),
-            "nulls_first" => Ok(Self::NullsFirst),
-            "nulls_last" => Ok(Self::NullsLast),
-            _ => plan_err!(
-                "Unknown null ordering: Expected one of 'nulls_first', 'nulls_last', 'nulls_min' or 'nulls_max'. Got {s}"
-            ),
-        }
-    }
-}
-
-impl From<&str> for NullOrdering {
-    fn from(s: &str) -> Self {
-        Self::from_str(s).unwrap_or(Self::NullsMax)
-    }
-}
-
 /// Ident Normalizer
 #[derive(Debug)]
 pub struct IdentNormalizer {

datafusion/sqllogictest/test_files/order.slt

@@ -158,26 +158,11 @@ SELECT * FROM (VALUES (1, 'one'), (2, 'two'), (null, 'three')) AS t (num,letter)
 1 one
 NULL three
 
-statement ok
+statement error Invalid default null ordering
 set datafusion.sql_parser.default_null_ordering = '';
 
-# test asc with an empty `default_null_ordering`. Expected to use the default null ordering which is `nulls_max`
-
-query IT
-SELECT * FROM (VALUES (1, 'one'), (2, 'two'), (null, 'three')) AS t (num,letter) ORDER BY num
-----
-1 one
-2 two
-NULL three
-
-# test desc with an empty `default_null_ordering`. Expected to use the default null ordering which is `nulls_max`
-
-query IT
-SELECT * FROM (VALUES (1, 'one'), (2, 'two'), (null, 'three')) AS t (num,letter) ORDER BY num DESC
-----
-NULL three
-2 two
-1 one
+statement error Invalid default null ordering: nuls_max
+set datafusion.sql_parser.default_null_ordering = 'nuls_max';
 
 statement error DataFusion error: Error during planning: Unsupported value Null
 set datafusion.sql_parser.default_null_ordering = null;

datafusion/sqllogictest/test_files/set_variable.slt

@@ -759,6 +759,14 @@ caused by
 Invalid or Unsupported Configuration: value must be greater than 0
 
 
+statement error
+SET datafusion.sql_parser.default_null_ordering = nuls_max
+----
+DataFusion error: Error setting config datafusion.sql_parser.default_null_ordering
+caused by
+Invalid or Unsupported Configuration: Invalid default null ordering: nuls_max. Expected one of: nulls_max, nulls_min, nulls_first, nulls_last
+
+
 # max_buffered_batches_per_output_file is halved to size an internal channel
 # capacity, so 0 and 1 both round down to a zero-capacity channel and must be
 # rejected, not just 0.