Skip to content

Commit 5bda9bd

Browse files
authored
fix: preserve case in generated field path expressions (#7698)
Fixes #7697 ## Summary Fix Lance-generated DataFusion field-path expressions so schema-derived column names preserve exact casing. Previously, `field_path_to_expr` used DataFusion `col(...)`, which lowercases unquoted identifiers. This caused generated nullable-vector filters like `VECTOR IS NOT NULL` to resolve as `vector`, breaking case-sensitive schemas during vector index creation / append optimization. ## Changes - Build root field-path expressions with `Expr::Column(Column::new_unqualified(...))` instead of `col(...)`. - Preserve existing nested field handling through `field_newstyle(...)`. - Add regression coverage for: - uppercase nullable vector column indexing / append optimization - exact-case root column expression generation - mixed-case root plus escaped nested field path containing dots ## Testing ```bash cargo test -p lance-datafusion logical_expr::tests::test_field_path_to_expr cargo test -p lance test_optimize_append_preserves_case_sensitive_nullable_vector_column cargo fmt --all <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved query parsing to correctly preserve case sensitivity for root column names and handle escaped nested field paths (including dots inside escaped segments). * Fixed an issue where appending data and optimizing indexes could leave unindexed fragments, affecting vector scans. * Vector search results now remain consistent after append + index optimization when using case-sensitive nullable vector columns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent a3d4719 commit 5bda9bd

2 files changed

Lines changed: 137 additions & 3 deletions

File tree

rust/lance-datafusion/src/logical_expr.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,10 @@ pub fn field_path_to_expr(field_path: &str) -> Result<Expr> {
281281
)));
282282
}
283283

284-
// Build the column expression, handling nested fields
285-
let mut expr = col(&parts[0]);
284+
// Build the column expression, handling nested fields.
285+
let mut expr = Expr::Column(datafusion::common::Column::new_unqualified(
286+
parts[0].clone(),
287+
));
286288
for part in &parts[1..] {
287289
expr = expr.field_newstyle(part);
288290
}
@@ -297,8 +299,26 @@ mod tests {
297299
use super::*;
298300

299301
use arrow_schema::{Field, Schema as ArrowSchema};
302+
use datafusion::common::Column;
300303
use datafusion_functions::core::expr_ext::FieldAccessor;
301304

305+
#[test]
306+
fn test_field_path_to_expr_preserves_case_sensitive_root_column() {
307+
let expr = field_path_to_expr("VECTOR").unwrap();
308+
309+
assert_eq!(expr, Expr::Column(Column::new_unqualified("VECTOR")));
310+
}
311+
312+
#[test]
313+
fn test_field_path_to_expr_preserves_case_sensitive_escaped_nested_path() {
314+
let expr = field_path_to_expr("Parent.`Child.With.Dot`").unwrap();
315+
316+
assert_eq!(
317+
expr,
318+
Expr::Column(Column::new_unqualified("Parent")).field_newstyle("Child.With.Dot")
319+
);
320+
}
321+
302322
#[test]
303323
fn test_resolve_large_utf8() {
304324
let arrow_schema = ArrowSchema::new(vec![Field::new("a", DataType::LargeUtf8, false)]);

rust/lance/src/index/append.rs

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -863,8 +863,10 @@ mod tests {
863863
use arrow::datatypes::{Float32Type, UInt32Type};
864864
use arrow_array::cast::AsArray;
865865
use arrow_array::{
866-
FixedSizeListArray, Int32Array, RecordBatch, RecordBatchIterator, StringArray, UInt32Array,
866+
ArrayRef, FixedSizeListArray, Int32Array, RecordBatch, RecordBatchIterator, StringArray,
867+
UInt32Array,
867868
};
869+
use arrow_buffer::{BooleanBufferBuilder, NullBuffer};
868870
use arrow_schema::{DataType, Field, Schema};
869871
use futures::TryStreamExt;
870872
use lance_arrow::FixedSizeListArrayExt;
@@ -1005,6 +1007,118 @@ mod tests {
10051007
assert_eq!(num_rows, 2000);
10061008
}
10071009

1010+
#[tokio::test]
1011+
async fn test_optimize_append_preserves_case_sensitive_nullable_vector_column() {
1012+
const DIM: usize = 64;
1013+
const ROWS: usize = 1000;
1014+
1015+
fn make_vectors(rows: usize, dim: usize, include_null: bool) -> FixedSizeListArray {
1016+
if include_null {
1017+
let mut nulls_builder = BooleanBufferBuilder::new(rows);
1018+
for row_idx in 0..rows {
1019+
nulls_builder.append(row_idx != 0);
1020+
}
1021+
let nulls = NullBuffer::new(nulls_builder.finish());
1022+
FixedSizeListArray::try_new(
1023+
Arc::new(Field::new("item", DataType::Float32, true)),
1024+
dim as i32,
1025+
Arc::new(generate_random_array(rows * dim)),
1026+
Some(nulls),
1027+
)
1028+
.unwrap()
1029+
} else {
1030+
FixedSizeListArray::try_new_from_values(
1031+
generate_random_array(rows * dim),
1032+
dim as i32,
1033+
)
1034+
.unwrap()
1035+
}
1036+
}
1037+
1038+
fn make_batch(
1039+
schema: Arc<Schema>,
1040+
start_id: u32,
1041+
vectors: Arc<FixedSizeListArray>,
1042+
) -> RecordBatch {
1043+
let columns: Vec<ArrayRef> = vec![
1044+
Arc::new(UInt32Array::from_iter_values(
1045+
start_id..start_id + ROWS as u32,
1046+
)) as ArrayRef,
1047+
vectors as ArrayRef,
1048+
];
1049+
RecordBatch::try_new(schema, columns).unwrap()
1050+
}
1051+
1052+
let test_dir = TempStrDir::default();
1053+
let test_uri = test_dir.as_str();
1054+
let vector_type = DataType::FixedSizeList(
1055+
Arc::new(Field::new("item", DataType::Float32, true)),
1056+
DIM as i32,
1057+
);
1058+
let schema = Arc::new(Schema::new(vec![
1059+
Field::new("id", DataType::UInt32, false),
1060+
Field::new("VECTOR", vector_type, true),
1061+
]));
1062+
1063+
let initial_vectors = Arc::new(make_vectors(ROWS, DIM, false));
1064+
let initial_batch = make_batch(schema.clone(), 0, initial_vectors);
1065+
let batches = RecordBatchIterator::new(std::iter::once(Ok(initial_batch)), schema.clone());
1066+
let mut dataset = Dataset::write(batches, test_uri, None).await.unwrap();
1067+
1068+
let params = VectorIndexParams::with_ivf_pq_params(
1069+
MetricType::L2,
1070+
IvfBuildParams::new(2),
1071+
PQBuildParams {
1072+
num_sub_vectors: 2,
1073+
..Default::default()
1074+
},
1075+
);
1076+
dataset
1077+
.create_index(&["VECTOR"], IndexType::Vector, None, &params, true)
1078+
.await
1079+
.unwrap();
1080+
1081+
let appended_vectors = Arc::new(make_vectors(ROWS, DIM, true));
1082+
let query = appended_vectors.value(5);
1083+
let appended_batch = make_batch(schema.clone(), ROWS as u32, appended_vectors);
1084+
let batches = RecordBatchIterator::new(std::iter::once(Ok(appended_batch)), schema);
1085+
dataset.append(batches, None).await.unwrap();
1086+
1087+
let index_name = dataset.load_indices().await.unwrap()[0].name.clone();
1088+
assert!(
1089+
!dataset
1090+
.unindexed_fragments(&index_name)
1091+
.await
1092+
.unwrap()
1093+
.is_empty()
1094+
);
1095+
1096+
dataset
1097+
.optimize_indices(&OptimizeOptions::append())
1098+
.await
1099+
.unwrap();
1100+
1101+
let dataset = DatasetBuilder::from_uri(test_uri).load().await.unwrap();
1102+
assert!(
1103+
dataset
1104+
.unindexed_fragments(&index_name)
1105+
.await
1106+
.unwrap()
1107+
.is_empty()
1108+
);
1109+
1110+
let mut scanner = dataset.scan();
1111+
scanner
1112+
.nearest("VECTOR", query.as_primitive::<Float32Type>(), 10)
1113+
.unwrap();
1114+
let results = scanner.try_into_batch().await.unwrap();
1115+
assert_eq!(
1116+
results.num_rows(),
1117+
10,
1118+
"expected the requested k=10 nearest-neighbor results"
1119+
);
1120+
}
1121+
10081122
/// Regression: a second `OptimizeOptions::append()` call on a steady-state
10091123
/// vector index used to fall through to `optimize_vector_indices` and write
10101124
/// a new UUID directory + manifest even though nothing had changed. The

0 commit comments

Comments
 (0)