Skip to content

Commit eb0eaa4

Browse files
zfarrellclaude
andcommitted
fix: reject column_id values exceeding i32 range
Replace unsafe `as i32` cast with `i32::try_from()` in Parquet field_id mapping. column_id > 2147483647 would silently wrap to negative, breaking column mapping. Now returns a clear error instead. Closes #63 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 22ea714 commit eb0eaa4

1 file changed

Lines changed: 71 additions & 1 deletion

File tree

src/types.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,12 @@ pub fn build_read_schema_with_field_id_mapping(
245245
.iter()
246246
.map(|col| {
247247
let data_type = ducklake_to_arrow_type(&col.column_type)?;
248-
let field_id = col.column_id as i32;
248+
let field_id = i32::try_from(col.column_id).map_err(|_| {
249+
DuckLakeError::Internal(format!(
250+
"column_id {} for column '{}' exceeds i32 range for Parquet field_id",
251+
col.column_id, col.column_name
252+
))
253+
})?;
249254

250255
let (read_name, needs_rename) =
251256
if let Some(parquet_name) = parquet_field_ids.get(&field_id) {
@@ -620,4 +625,69 @@ mod tests {
620625
_ => panic!("Expected UnsupportedType error when building schema with complex type"),
621626
}
622627
}
628+
629+
#[test]
630+
fn test_column_id_i32_max_succeeds() {
631+
let columns = vec![DuckLakeTableColumn {
632+
column_id: i32::MAX as i64,
633+
column_name: "id".to_string(),
634+
column_type: "int32".to_string(),
635+
is_nullable: true,
636+
}];
637+
638+
let mut parquet_field_ids = HashMap::new();
639+
parquet_field_ids.insert(i32::MAX, "id".to_string());
640+
641+
let result = build_read_schema_with_field_id_mapping(&columns, &parquet_field_ids);
642+
assert!(result.is_ok(), "column_id = i32::MAX should succeed");
643+
}
644+
645+
#[test]
646+
fn test_column_id_overflow_returns_error() {
647+
let columns = vec![DuckLakeTableColumn {
648+
column_id: i32::MAX as i64 + 1, // 2147483648, exceeds i32 range
649+
column_name: "id".to_string(),
650+
column_type: "int32".to_string(),
651+
is_nullable: true,
652+
}];
653+
654+
let parquet_field_ids = HashMap::new();
655+
656+
let result = build_read_schema_with_field_id_mapping(&columns, &parquet_field_ids);
657+
assert!(result.is_err(), "column_id > i32::MAX should fail");
658+
match result {
659+
Err(DuckLakeError::Internal(msg)) => {
660+
assert!(
661+
msg.contains("2147483648"),
662+
"Error should contain the overflowing value: {}",
663+
msg
664+
);
665+
assert!(
666+
msg.contains("exceeds i32 range"),
667+
"Error should explain the issue: {}",
668+
msg
669+
);
670+
},
671+
_ => panic!("Expected Internal error for column_id overflow"),
672+
}
673+
}
674+
675+
#[test]
676+
fn test_column_id_negative_within_i32_range_succeeds() {
677+
let columns = vec![DuckLakeTableColumn {
678+
column_id: -1,
679+
column_name: "id".to_string(),
680+
column_type: "int32".to_string(),
681+
is_nullable: true,
682+
}];
683+
684+
let mut parquet_field_ids = HashMap::new();
685+
parquet_field_ids.insert(-1_i32, "id".to_string());
686+
687+
let result = build_read_schema_with_field_id_mapping(&columns, &parquet_field_ids);
688+
assert!(
689+
result.is_ok(),
690+
"Negative column_id within i32 range should succeed"
691+
);
692+
}
623693
}

0 commit comments

Comments
 (0)