Skip to content

Commit 27351e5

Browse files
authored
fix: keep delete-by-source fast path with scalar indexes (#6435)
This teaches `merge_insert` to keep the delete-by-source fast path even when a scalar index exists on the join key. The actual indexed join path is still only used when unmatched target rows are kept, so the presence of index metadata should not force these operations back to the legacy full-join path. This also adds regression coverage for full-schema `FixedSizeList` merges with `when_not_matched_by_source(Delete)` both with and without a scalar index. That closes the gap behind #6195 and preserves the earlier fix for lancedb/lancedb#3094.
1 parent dabfb93 commit 27351e5

1 file changed

Lines changed: 188 additions & 15 deletions

File tree

rust/lance/src/dataset/write/merge_insert.rs

Lines changed: 188 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -864,25 +864,28 @@ impl MergeInsertJob {
864864
&self,
865865
source: SendableRecordBatchStream,
866866
) -> Result<SendableRecordBatchStream> {
867-
// We need to do a full index scan if we're deleting source data
868-
let can_use_scalar_index = matches!(
869-
self.params.delete_not_matched_by_source, // this value marks behavior for rows in target that are not matched by the source. Value assigned earlier.
870-
WhenNotMatchedBySource::Keep
871-
) && self.params.use_index;
872-
873-
if can_use_scalar_index {
867+
if self.params.use_index
868+
&& matches!(
869+
self.params.delete_not_matched_by_source,
870+
WhenNotMatchedBySource::Keep
871+
)
872+
{
874873
// keeping unmatched rows, no deletion
875874
if let Some(index) = self.join_key_as_scalar_index().await? {
876-
self.create_indexed_scan_joined_stream(source, index).await
877-
} else {
878-
self.create_full_table_joined_stream(source).await
875+
return self.create_indexed_scan_joined_stream(source, index).await;
879876
}
880-
} else {
877+
}
878+
879+
if !matches!(
880+
self.params.delete_not_matched_by_source,
881+
WhenNotMatchedBySource::Keep
882+
) {
881883
info!(
882884
"The merge insert operation is configured to delete rows from the target table, this requires a potentially costly full table scan"
883885
);
884-
self.create_full_table_joined_stream(source).await
885886
}
887+
888+
self.create_full_table_joined_stream(source).await
886889
}
887890

888891
async fn update_fragments(
@@ -1480,7 +1483,7 @@ impl MergeInsertJob {
14801483
///
14811484
/// The fast path is only available for specific conditions:
14821485
/// - when_matched is UpdateAll or UpdateIf or Fail
1483-
/// - Either use_index is false OR there's no scalar index on join key
1486+
/// - The execution will not use the legacy scalar-index join path
14841487
/// - Source schema matches dataset schema exactly
14851488
/// - when_not_matched_by_source is Keep, Delete, or DeleteIf
14861489
async fn can_use_create_plan(&self, source_schema: &Schema) -> Result<bool> {
@@ -1499,7 +1502,15 @@ impl MergeInsertJob {
14991502
},
15001503
);
15011504

1502-
let has_scalar_index = self.join_key_as_scalar_index().await?.is_some();
1505+
let would_use_scalar_index = if self.params.use_index
1506+
&& matches!(
1507+
self.params.delete_not_matched_by_source,
1508+
WhenNotMatchedBySource::Keep
1509+
) {
1510+
self.join_key_as_scalar_index().await?.is_some()
1511+
} else {
1512+
false
1513+
};
15031514

15041515
// Check if this is a delete-only operation (no update/insert writes needed from source)
15051516
// For delete-only, we don't need the full source schema, just key columns for matching
@@ -1523,7 +1534,7 @@ impl MergeInsertJob {
15231534
| WhenMatched::UpdateIf(_)
15241535
| WhenMatched::Fail
15251536
| WhenMatched::Delete
1526-
) && (!self.params.use_index || !has_scalar_index)
1537+
) && !would_use_scalar_index
15271538
&& schema_ok
15281539
&& matches!(
15291540
self.params.delete_not_matched_by_source,
@@ -5448,6 +5459,76 @@ MergeInsert: on=[id], when_matched=UpdateAll, when_not_matched=InsertAll, when_n
54485459
);
54495460
}
54505461

5462+
#[tokio::test]
5463+
async fn test_explain_plan_full_schema_delete_by_source_with_fsl_and_scalar_index() {
5464+
let schema = Arc::new(Schema::new(vec![
5465+
Field::new("id", DataType::Int32, false),
5466+
Field::new(
5467+
"vec",
5468+
DataType::FixedSizeList(Arc::new(Field::new("item", DataType::Float32, true)), 4),
5469+
true,
5470+
),
5471+
]));
5472+
5473+
let dataset_batch = RecordBatch::try_new(
5474+
schema.clone(),
5475+
vec![
5476+
Arc::new(Int32Array::from(vec![1, 2, 3])),
5477+
Arc::new(
5478+
FixedSizeListArray::try_new_from_values(
5479+
Float32Array::from(vec![
5480+
1.0, 1.1, 1.2, 1.3, 2.0, 2.1, 2.2, 2.3, 3.0, 3.1, 3.2, 3.3,
5481+
]),
5482+
4,
5483+
)
5484+
.unwrap(),
5485+
),
5486+
],
5487+
)
5488+
.unwrap();
5489+
5490+
let mut dataset = Dataset::write(
5491+
Box::new(RecordBatchIterator::new(
5492+
[Ok(dataset_batch)],
5493+
schema.clone(),
5494+
)),
5495+
"memory://test_explain_plan_full_schema_delete_by_source_with_fsl_and_scalar_index",
5496+
None,
5497+
)
5498+
.await
5499+
.unwrap();
5500+
5501+
let scalar_params = ScalarIndexParams::default();
5502+
dataset
5503+
.create_index(&["id"], IndexType::Scalar, None, &scalar_params, false)
5504+
.await
5505+
.unwrap();
5506+
5507+
let merge_insert_job =
5508+
MergeInsertBuilder::try_new(Arc::new(dataset), vec!["id".to_string()])
5509+
.unwrap()
5510+
.when_matched(WhenMatched::UpdateAll)
5511+
.when_not_matched(WhenNotMatched::InsertAll)
5512+
.when_not_matched_by_source(WhenNotMatchedBySource::Delete)
5513+
.try_build()
5514+
.unwrap();
5515+
5516+
let plan = merge_insert_job.explain_plan(None, false).await.unwrap();
5517+
assert!(plan.contains("HashJoinExec"));
5518+
assert!(plan.contains("join_type=Full"));
5519+
assert!(plan.contains("projection=[_rowid"));
5520+
assert!(
5521+
plan.contains("LanceRead: uri=") && plan.contains("projection=[id]"),
5522+
"target-side scan should prune the FSL payload from the join build side even when a scalar index exists: {plan}"
5523+
);
5524+
assert!(
5525+
!plan.contains(
5526+
"LanceRead: uri=test_explain_plan_full_schema_delete_by_source_with_fsl_and_scalar_index/data, projection=[id, vec]"
5527+
),
5528+
"target-side scan should not include the FSL payload in the join build side: {plan}"
5529+
);
5530+
}
5531+
54515532
#[tokio::test]
54525533
async fn test_merge_insert_full_schema_delete_by_source_with_fsl() {
54535534
let schema = Arc::new(Schema::new(vec![
@@ -5534,6 +5615,98 @@ MergeInsert: on=[id], when_matched=UpdateAll, when_not_matched=InsertAll, when_n
55345615
assert_eq!(actual, vec![20.0, 20.1, 20.2, 20.3, 40.0, 40.1, 40.2, 40.3]);
55355616
}
55365617

5618+
#[tokio::test]
5619+
async fn test_merge_insert_full_schema_delete_by_source_with_fsl_and_scalar_index() {
5620+
let schema = Arc::new(Schema::new(vec![
5621+
Field::new("id", DataType::Int32, false),
5622+
Field::new(
5623+
"vec",
5624+
DataType::FixedSizeList(Arc::new(Field::new("item", DataType::Float32, true)), 4),
5625+
true,
5626+
),
5627+
]));
5628+
5629+
let dataset_batch = RecordBatch::try_new(
5630+
schema.clone(),
5631+
vec![
5632+
Arc::new(Int32Array::from(vec![1, 2, 3])),
5633+
Arc::new(
5634+
FixedSizeListArray::try_new_from_values(
5635+
Float32Array::from(vec![
5636+
1.0, 1.1, 1.2, 1.3, 2.0, 2.1, 2.2, 2.3, 3.0, 3.1, 3.2, 3.3,
5637+
]),
5638+
4,
5639+
)
5640+
.unwrap(),
5641+
),
5642+
],
5643+
)
5644+
.unwrap();
5645+
5646+
let mut dataset = Dataset::write(
5647+
Box::new(RecordBatchIterator::new(
5648+
[Ok(dataset_batch)],
5649+
schema.clone(),
5650+
)),
5651+
"memory://test_merge_insert_full_schema_delete_by_source_with_fsl_and_scalar_index",
5652+
None,
5653+
)
5654+
.await
5655+
.unwrap();
5656+
5657+
let scalar_params = ScalarIndexParams::default();
5658+
dataset
5659+
.create_index(&["id"], IndexType::Scalar, None, &scalar_params, false)
5660+
.await
5661+
.unwrap();
5662+
5663+
let source_batch = RecordBatch::try_new(
5664+
schema.clone(),
5665+
vec![
5666+
Arc::new(Int32Array::from(vec![2, 4])),
5667+
Arc::new(
5668+
FixedSizeListArray::try_new_from_values(
5669+
Float32Array::from(vec![20.0, 20.1, 20.2, 20.3, 40.0, 40.1, 40.2, 40.3]),
5670+
4,
5671+
)
5672+
.unwrap(),
5673+
),
5674+
],
5675+
)
5676+
.unwrap();
5677+
5678+
let (merged_dataset, stats) =
5679+
MergeInsertBuilder::try_new(Arc::new(dataset), vec!["id".to_string()])
5680+
.unwrap()
5681+
.when_matched(WhenMatched::UpdateAll)
5682+
.when_not_matched(WhenNotMatched::InsertAll)
5683+
.when_not_matched_by_source(WhenNotMatchedBySource::Delete)
5684+
.try_build()
5685+
.unwrap()
5686+
.execute_reader(Box::new(RecordBatchIterator::new(
5687+
[Ok(source_batch)],
5688+
schema.clone(),
5689+
)))
5690+
.await
5691+
.unwrap();
5692+
5693+
assert_eq!(stats.num_deleted_rows, 2);
5694+
assert_eq!(stats.num_updated_rows, 1);
5695+
assert_eq!(stats.num_inserted_rows, 1);
5696+
5697+
let merged = merged_dataset.scan().try_into_batch().await.unwrap();
5698+
let ids = merged["id"].as_primitive::<Int32Type>().values().to_vec();
5699+
assert_eq!(ids, vec![2, 4]);
5700+
5701+
let vecs = merged["vec"].as_fixed_size_list();
5702+
let actual = vecs
5703+
.values()
5704+
.as_primitive::<Float32Type>()
5705+
.values()
5706+
.to_vec();
5707+
assert_eq!(actual, vec![20.0, 20.1, 20.2, 20.3, 40.0, 40.1, 40.2, 40.3]);
5708+
}
5709+
55375710
#[tokio::test]
55385711
async fn test_analyze_plan() {
55395712
// Set up test data using lance_datagen

0 commit comments

Comments
 (0)