feat: Rename metadata columns to use underscore prefix (_location, _last_modified, _size) (#138)

Author: phillipleblanc

datafusion/datasource/src/file_scan_config.rs

@@ -2834,9 +2834,9 @@ mod tests {
         // Verify the schema matches expected order
         assert_eq!(result.num_columns(), 5);
         assert_eq!(result.schema().field(0).name(), "data");
-        assert_eq!(result.schema().field(1).name(), "location");
+        assert_eq!(result.schema().field(1).name(), "_location");
         assert_eq!(result.schema().field(2).name(), "year");
-        assert_eq!(result.schema().field(3).name(), "size");
+        assert_eq!(result.schema().field(3).name(), "_size");
         assert_eq!(result.schema().field(4).name(), "month");
 
         // Verify the values are correct in each column
@@ -2916,7 +2916,7 @@ mod tests {
 
         assert_eq!(result.num_columns(), 3);
         assert_eq!(result.schema().field(0).name(), "value");
-        assert_eq!(result.schema().field(1).name(), "size");
+        assert_eq!(result.schema().field(1).name(), "_size");
         assert_eq!(result.schema().field(2).name(), "part");
 
         // Verify values
@@ -2981,7 +2981,7 @@ mod tests {
         assert_eq!(result.num_columns(), 3);
         assert_eq!(result.schema().field(0).name(), "value");
         assert_eq!(result.schema().field(1).name(), "part");
-        assert_eq!(result.schema().field(2).name(), "location");
+        assert_eq!(result.schema().field(2).name(), "_location");
 
         let location_col = result
             .column(2)
@@ -3065,10 +3065,10 @@ mod tests {
         // Verify schema order
         assert_eq!(result.num_columns(), 7);
         assert_eq!(result.schema().field(0).name(), "col_a");
-        assert_eq!(result.schema().field(1).name(), "size");
+        assert_eq!(result.schema().field(1).name(), "_size");
         assert_eq!(result.schema().field(2).name(), "col_b");
         assert_eq!(result.schema().field(3).name(), "p1");
-        assert_eq!(result.schema().field(4).name(), "last_modified");
+        assert_eq!(result.schema().field(4).name(), "_last_modified");
         assert_eq!(result.schema().field(5).name(), "col_c");
         assert_eq!(result.schema().field(6).name(), "p2");
 
@@ -3212,8 +3212,8 @@ mod tests {
 
         assert_eq!(result.num_columns(), 3);
         assert_eq!(result.schema().field(0).name(), "data");
-        assert_eq!(result.schema().field(1).name(), "location");
-        assert_eq!(result.schema().field(2).name(), "size");
+        assert_eq!(result.schema().field(1).name(), "_location");
+        assert_eq!(result.schema().field(2).name(), "_size");
 
         let location_col = result
             .column(1)
@@ -3280,7 +3280,7 @@ mod tests {
         assert_eq!(result.num_columns(), 2);
         assert_eq!(result.num_rows(), 5);
         assert_eq!(result.schema().field(0).name(), "part");
-        assert_eq!(result.schema().field(1).name(), "size");
+        assert_eq!(result.schema().field(1).name(), "_size");
     }
 
     #[test]
@@ -3759,12 +3759,12 @@ mod tests {
             .projected_schema()
             .expect("projected schema");
         assert_eq!(projected.fields().len(), 3);
-        assert_eq!(projected.field(2).name(), "location");
+        assert_eq!(projected.field(2).name(), "_location");
 
         // Create a filter on the metadata column: location@2 = 's3://bucket'
         // (index 2 because projected output is [id@0, value@1, location@2])
         let location_filter: Arc<dyn PhysicalExpr> = Arc::new(BinaryExpr::new(
-            Arc::new(Column::new("location", 2)),
+            Arc::new(Column::new("_location", 2)),
             Operator::Eq,
             Arc::new(Literal::new(ScalarValue::Utf8(Some(
                 "s3://bucket".to_string(),
@@ -3825,7 +3825,7 @@ mod tests {
         ));
         // Filter 1: metadata column filter on location@2 (beyond projection)
         let location_filter: Arc<dyn PhysicalExpr> = Arc::new(BinaryExpr::new(
-            Arc::new(Column::new("location", 2)),
+            Arc::new(Column::new("_location", 2)),
             Operator::Eq,
             Arc::new(Literal::new(ScalarValue::Utf8(Some(
                 "s3://bucket".to_string(),
@@ -3884,8 +3884,8 @@ mod tests {
         );
         assert_eq!(schema.field(0).name(), "id");
         assert_eq!(schema.field(1).name(), "value");
-        assert_eq!(schema.field(2).name(), "location");
-        assert_eq!(schema.field(3).name(), "size");
+        assert_eq!(schema.field(2).name(), "_location");
+        assert_eq!(schema.field(3).name(), "_size");
     }
 
     /// Same as above but with a projection applied — metadata columns must
@@ -3932,7 +3932,7 @@ mod tests {
         );
         assert_eq!(schema.field(0).name(), "id");
         assert_eq!(schema.field(1).name(), "value");
-        assert_eq!(schema.field(2).name(), "location");
+        assert_eq!(schema.field(2).name(), "_location");
 
         // Verify it matches projected_schema()
         let projected = data_source
@@ -4043,7 +4043,7 @@ mod tests {
         // metadata = [location(2), size(3)]
         // SELECT location, id -> projection = [2, 0]
         // -> file_partition_indices = [0], metadata_positions = [(0, 0)]
-        // Metadata "location" should be at output position 0, "id" after it.
+        // Metadata "_location" should be at output position 0, "id" after it.
         let config = FileScanConfigBuilder::new(object_store_url, file_source)
             .with_metadata_cols(metadata_cols)
             .with_projection_indices(Some(vec![2, 0]))
@@ -4151,9 +4151,9 @@ mod tests {
         assert_eq!(projected.field(1).name(), "year");
         assert_eq!(projected.field(2).name(), "month");
         assert_eq!(projected.field(3).name(), "day");
-        assert_eq!(projected.field(4).name(), "location");
-        assert_eq!(projected.field(5).name(), "size");
-        assert_eq!(projected.field(6).name(), "last_modified");
+        assert_eq!(projected.field(4).name(), "_location");
+        assert_eq!(projected.field(5).name(), "_size");
+        assert_eq!(projected.field(6).name(), "_last_modified");
     }
 
     /// Test that projection with only some metadata columns works correctly.
@@ -4206,7 +4206,7 @@ mod tests {
         );
 
         assert_eq!(projected.field(0).name(), "id");
-        assert_eq!(projected.field(1).name(), "location");
+        assert_eq!(projected.field(1).name(), "_location");
     }
 
     /// Test that projection with metadata columns in non-sequential order works.
@@ -4253,9 +4253,9 @@ mod tests {
             projected.fields().len()
         );
 
-        assert_eq!(projected.field(0).name(), "location");
+        assert_eq!(projected.field(0).name(), "_location");
         assert_eq!(projected.field(1).name(), "id");
-        assert_eq!(projected.field(2).name(), "size");
+        assert_eq!(projected.field(2).name(), "_size");
     }
 
     /// Test partial projection selecting file, partition, AND metadata columns.
@@ -4311,7 +4311,7 @@ mod tests {
 
         assert_eq!(projected.field(0).name(), "id");
         assert_eq!(projected.field(1).name(), "year");
-        assert_eq!(projected.field(2).name(), "location");
+        assert_eq!(projected.field(2).name(), "_location");
 
         // Now test reordered: SELECT location, year, id -> projection = [4, 2, 0]
         let file_source2: Arc<dyn FileSource> =
@@ -4335,7 +4335,7 @@ mod tests {
             projected2.fields().len()
         );
 
-        assert_eq!(projected2.field(0).name(), "location");
+        assert_eq!(projected2.field(0).name(), "_location");
         assert_eq!(projected2.field(1).name(), "year");
         assert_eq!(projected2.field(2).name(), "id");
     }
@@ -4380,7 +4380,7 @@ mod tests {
             "Expected 1 column (location only), got {}",
             projected.fields().len()
         );
-        assert_eq!(projected.field(0).name(), "location");
+        assert_eq!(projected.field(0).name(), "_location");
 
         // The file source should have an empty projection (no file/partition columns)
         let source_proj = config
@@ -4398,7 +4398,7 @@ mod tests {
         let binding = config.eq_properties();
         let eq_schema = binding.schema();
         assert_eq!(eq_schema.fields().len(), 1);
-        assert_eq!(eq_schema.field(0).name(), "location");
+        assert_eq!(eq_schema.field(0).name(), "_location");
     }
 
     /// Test that with_projection_indices correctly handles a mix of file,
@@ -4433,7 +4433,7 @@ mod tests {
         let projected = config.projected_schema().expect("projected schema");
         assert_eq!(projected.fields().len(), 3);
         assert_eq!(projected.field(0).name(), "value");
-        assert_eq!(projected.field(1).name(), "location");
+        assert_eq!(projected.field(1).name(), "_location");
         assert_eq!(projected.field(2).name(), "year");
 
         // File source should have projection for indices [1, 2] (value, year)

datafusion/datasource/src/file_stream.rs

@@ -1055,8 +1055,8 @@ mod tests {
         let schema_with_metadata = Arc::new(Schema::new(vec![
             Field::new("id", DataType::Int32, false),
             Field::new("value", DataType::Utf8, true),
-            Field::new("location", DataType::Utf8, true),
-            Field::new("size", DataType::UInt64, true),
+            Field::new("_location", DataType::Utf8, true),
+            Field::new("_size", DataType::UInt64, true),
         ]));
 
         // Create the projector with metadata columns
@@ -1093,8 +1093,8 @@ mod tests {
             Field::new("id", DataType::Int32, false),
             Field::new("value", DataType::Utf8, true),
             Field::new("year", DataType::Utf8, true),
-            Field::new("location", DataType::Utf8, true),
-            Field::new("size", DataType::UInt64, true),
+            Field::new("_location", DataType::Utf8, true),
+            Field::new("_size", DataType::UInt64, true),
         ]));
 
         // Create the projector
@@ -1130,11 +1130,11 @@ mod tests {
 
         // Create a schema with columns in a different order
         let schema_mixed = Arc::new(Schema::new(vec![
-            Field::new("location", DataType::Utf8, true), // metadata column first
+            Field::new("_location", DataType::Utf8, true), // metadata column first
             Field::new("id", DataType::Int32, false),     // file column
             Field::new("year", DataType::Utf8, true),     // partition column
             Field::new("value", DataType::Utf8, true),    // file column
-            Field::new("size", DataType::UInt64, true),   // metadata column last
+            Field::new("_size", DataType::UInt64, true),   // metadata column last
         ]));
 
         // Create the projector
@@ -1174,7 +1174,7 @@ mod tests {
             .collect();
 
         // Check location column
-        let location_idx = *field_indices.get("location").unwrap();
+        let location_idx = *field_indices.get("_location").unwrap();
         let location_col = projected_batch.column(location_idx);
         let location_array = location_col.as_any().downcast_ref::<StringArray>().unwrap();
         assert_eq!(location_array.value(0), "test/file.parquet");
@@ -1201,7 +1201,7 @@ mod tests {
         assert_eq!(value_array.value(0), "a");
 
         // Check size column
-        let size_idx = *field_indices.get("size").unwrap();
+        let size_idx = *field_indices.get("_size").unwrap();
         let size_col = projected_batch.column(size_idx);
         let size_array = size_col.as_any().downcast_ref::<UInt64Array>().unwrap();
         assert_eq!(size_array.value(0), 1024);

datafusion/datasource/src/metadata.rs

@@ -50,12 +50,12 @@ impl fmt::Display for MetadataColumn {
 }
 
 impl MetadataColumn {
-    /// The name of the metadata column (one of `location`, `last_modified`, or `size`)
+    /// The name of the metadata column (one of `_location`, `_last_modified`, or `_size`)
     pub fn name(&self) -> &str {
         match self {
-            MetadataColumn::Location(_) => "location",
-            MetadataColumn::LastModified => "last_modified",
-            MetadataColumn::Size => "size",
+            MetadataColumn::Location(_) => "_location",
+            MetadataColumn::LastModified => "_last_modified",
+            MetadataColumn::Size => "_size",
         }
     }
 
@@ -114,11 +114,11 @@ impl FromStr for MetadataColumn {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
-            "location" => Ok(MetadataColumn::Location(None)),
-            "last_modified" => Ok(MetadataColumn::LastModified),
-            "size" => Ok(MetadataColumn::Size),
+            "_location" => Ok(MetadataColumn::Location(None)),
+            "_last_modified" => Ok(MetadataColumn::LastModified),
+            "_size" => Ok(MetadataColumn::Size),
             _ => plan_err!(
-                "Invalid metadata column: {}, expected: location, last_modified, or size",
+                "Invalid metadata column: {}, expected: _location, _last_modified, or _size",
                 s
             ),
         }
@@ -180,9 +180,9 @@ mod tests {
 
     #[test]
     fn test_metadata_column_name() {
-        assert_eq!(MetadataColumn::Location(None).name(), "location");
-        assert_eq!(MetadataColumn::LastModified.name(), "last_modified");
-        assert_eq!(MetadataColumn::Size.name(), "size");
+        assert_eq!(MetadataColumn::Location(None).name(), "_location");
+        assert_eq!(MetadataColumn::LastModified.name(), "_last_modified");
+        assert_eq!(MetadataColumn::Size.name(), "_size");
     }
 
     #[test]
@@ -198,20 +198,20 @@ mod tests {
     #[test]
     fn test_metadata_column_field() {
         let field = MetadataColumn::Location(None).field();
-        assert_eq!(field.name(), "location");
+        assert_eq!(field.name(), "_location");
         assert_eq!(field.data_type(), &DataType::Utf8);
         assert!(field.is_nullable());
 
         let field = MetadataColumn::LastModified.field();
-        assert_eq!(field.name(), "last_modified");
+        assert_eq!(field.name(), "_last_modified");
         assert_eq!(
             field.data_type(),
             &DataType::Timestamp(TimeUnit::Microsecond, Some("UTC".into()))
         );
         assert!(field.is_nullable());
 
         let field = MetadataColumn::Size.field();
-        assert_eq!(field.name(), "size");
+        assert_eq!(field.name(), "_size");
         assert_eq!(field.data_type(), &DataType::UInt64);
         assert!(field.is_nullable());
     }
@@ -247,15 +247,15 @@ mod tests {
     fn test_metadata_column_from_str() {
         // Test valid values
         assert_eq!(
-            MetadataColumn::from_str("location").unwrap(),
+            MetadataColumn::from_str("_location").unwrap(),
             MetadataColumn::Location(None)
         );
         assert_eq!(
-            MetadataColumn::from_str("last_modified").unwrap(),
+            MetadataColumn::from_str("_last_modified").unwrap(),
             MetadataColumn::LastModified
         );
         assert_eq!(
-            MetadataColumn::from_str("size").unwrap(),
+            MetadataColumn::from_str("_size").unwrap(),
             MetadataColumn::Size
         );
 
@@ -266,9 +266,9 @@ mod tests {
 
     #[test]
     fn test_metadata_column_display() {
-        assert_eq!(format!("{}", MetadataColumn::Location(None)), "location");
-        assert_eq!(format!("{}", MetadataColumn::LastModified), "last_modified");
-        assert_eq!(format!("{}", MetadataColumn::Size), "size");
+        assert_eq!(format!("{}", MetadataColumn::Location(None)), "_location");
+        assert_eq!(format!("{}", MetadataColumn::LastModified), "_last_modified");
+        assert_eq!(format!("{}", MetadataColumn::Size), "_size");
     }
 
     #[test]