parse table metadata.configuration as TableProperties

kernel/src/table_properties.rs

@@ -11,6 +11,7 @@
 //! table.
 
 use std::collections::HashMap;
+use std::num::NonZero;
 use std::time::Duration;
 
 use crate::expressions::ColumnName;
@@ -43,7 +44,7 @@ pub struct TableProperties {
 
     /// Interval (expressed as number of commits) after which a new checkpoint should be created.
     /// E.g. if checkpoint interval = 10, then a checkpoint should be written every 10 commits.
-    pub checkpoint_interval: Option<i64>,
+    pub checkpoint_interval: Option<NonZero<u64>>,
 
     /// true for Delta Lake to write file statistics in checkpoints in JSON format for the stats column.
     pub checkpoint_write_stats_as_json: Option<bool>,
@@ -114,7 +115,7 @@ pub struct TableProperties {
 
     /// When delta.randomizeFilePrefixes is set to true, the number of characters that Delta
     /// generates for random prefixes.
-    pub random_prefix_length: Option<i64>,
+    pub random_prefix_length: Option<NonZero<u64>>,
 
     /// The shortest duration within which new snapshots will retain transaction identifiers (for
     /// example, SetTransactions). When a new snapshot sees a transaction identifier older than or
@@ -124,7 +125,7 @@ pub struct TableProperties {
 
     /// The target file size in bytes or higher units for file tuning. For example, 104857600
     /// (bytes) or 100mb.
-    pub target_file_size: Option<i64>,
+    pub target_file_size: Option<NonZero<u64>>,
 
     /// The target file size in bytes or higher units for file tuning. For example, 104857600
     /// (bytes) or 100mb.
@@ -273,7 +274,7 @@ mod tests {
             append_only: Some(true),
             optimize_write: Some(true),
             auto_compact: Some(true),
-            checkpoint_interval: Some(101),
+            checkpoint_interval: Some(NonZero::new(101).unwrap()),
             checkpoint_write_stats_as_json: Some(true),
             checkpoint_write_stats_as_struct: Some(true),
             column_mapping_mode: Some(ColumnMappingMode::Id),
@@ -286,9 +287,9 @@ mod tests {
             log_retention_duration: Some(Duration::new(2, 0)),
             enable_expired_log_cleanup: Some(true),
             randomize_file_prefixes: Some(true),
-            random_prefix_length: Some(1001),
+            random_prefix_length: Some(NonZero::new(1001).unwrap()),
             set_transaction_retention_duration: Some(Duration::new(60, 0)),
-            target_file_size: Some(1_000_000_000),
+            target_file_size: Some(NonZero::new(1_000_000_000).unwrap()),
             tune_file_sizes_for_rewrites: Some(true),
             checkpoint_policy: Some(CheckpointPolicy::V2),
             enable_row_tracking: Some(true),

kernel/src/table_properties/deserialize.rs

@@ -2,13 +2,16 @@
 //! us to relatively simply implement the functionality described in the protocol and expose
 //! 'simple' types to the user in the [`TableProperties`] struct. E.g. we can expose a `bool`
 //! directly instead of a `BoolConfig` type that we implement `Deserialize` for.
+use std::num::NonZero;
 use std::time::Duration;
 
 use super::*;
 use crate::expressions::ColumnName;
 use crate::table_features::ColumnMappingMode;
 use crate::utils::require;
 
+use tracing::warn;
+
 const SECONDS_PER_MINUTE: u64 = 60;
 const SECONDS_PER_HOUR: u64 = 60 * SECONDS_PER_MINUTE;
 const SECONDS_PER_DAY: u64 = 24 * SECONDS_PER_HOUR;
@@ -80,10 +83,10 @@ fn try_parse(props: &mut TableProperties, k: &str, v: &str) -> Option<()> {
 
 /// Deserialize a string representing a positive integer into an `Option<u64>`. Returns `Some` if
 /// successfully parses, and `None` otherwise.
-pub(crate) fn parse_positive_int(s: &str) -> Option<i64> {
+pub(crate) fn parse_positive_int(s: &str) -> Option<NonZero<u64>> {
     // parse to i64 (then check n > 0) since java doesn't even allow u64
     let n: i64 = s.parse().ok()?;
-    (n > 0).then_some(n)
+    NonZero::new(n.try_into().ok()?)
 }
 
 /// Deserialize a string representing a boolean into an `Option<bool>`. Returns `Some` if
@@ -99,7 +102,9 @@ pub(crate) fn parse_bool(s: &str) -> Option<bool> {
 /// Deserialize a comma-separated list of column names into an `Option<Vec<ColumnName>>`. Returns
 /// `Some` if successfully parses, and `None` otherwise.
 pub(crate) fn parse_column_names(s: &str) -> Option<Vec<ColumnName>> {
-    ColumnName::parse_column_name_list(s).ok()
+    ColumnName::parse_column_name_list(s)
+        .map_err(|e| warn!("column name list failed to parse: {e}"))
+        .ok()
 }
 
 /// Deserialize an interval string of the form "interval 5 days" into an `Option<Duration>`.
@@ -185,6 +190,33 @@ fn parse_interval_impl(value: &str) -> Result<Duration, ParseIntervalError> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::expressions::column_name;
+
+    #[test]
+    fn test_parse_column_names() {
+        assert_eq!(
+            parse_column_names("`col 1`, col.a2,col3").unwrap(),
+            vec![
+                ColumnName::new(["col 1"]),
+                column_name!("col.a2"),
+                column_name!("col3")
+            ]
+        );
+    }
+
+    #[test]
+    fn test_parse_bool() {
+        assert_eq!(parse_bool("true").unwrap(), true);
+        assert_eq!(parse_bool("false").unwrap(), false);
+        assert_eq!(parse_bool("whatever"), None);
+    }
+
+    #[test]
+    fn test_parse_positive_int() {
+        assert_eq!(parse_positive_int("123").unwrap().get(), 123);
+        assert_eq!(parse_positive_int("0"), None);
+        assert_eq!(parse_positive_int("-123"), None);
+    }
 
     #[test]
     fn test_parse_interval() {