Skip to content

fix(datafusion): honor ORDER BY and LIMIT in UPDATE and DELETE - #1397

Open
lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:fix/dml-selection-clauses
Open

lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:fix/dml-selection-clauses

Conversation

@lyne7-sc

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #1396

Paimon's DataFusion SQLContext ignores ORDER BY and LIMIT when selecting rows for UPDATE and DELETE, which can modify more rows than requested. This change honors these clauses for both Copy-on-Write and Data Evolution tables.

Brief change log

Preserve WHERE, ORDER BY and LIMIT in Copy-on-Write and Data Evolution selection queries, applying sorting and limiting before writing.

Tests

test_update_with_order_by_and_limit and test_delete_with_order_by_and_limit cover ordered selection and LIMIT on both paths.

API and Format

No public API or storage format changes.

Documentation

No documentation changes; this fixes execution of already parsed clauses.

};
let selection_clause = build_dml_selection_clause(
update.selection.as_ref(),
&update.order_by,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the table has a column named __upd_id, UPDATE t SET id = id + 100 ORDER BY __upd_id LIMIT 1 sorts by the generated SET alias. I tried this with (id, __upd_id) values (1, 20) and (2, 10). Both paths updated row 1, but row 2 has the smaller __upd_id. Can we select the target rows before projecting the SET expressions?

// Selection clauses are rendered from the sqlparser AST.
let query_sql = format!(
"SELECT \"__paimon_file_idx\", \"__paimon_row_offset\" FROM {cow_table_name}{where_clause}"
"SELECT \"__paimon_file_idx\", \"__paimon_row_offset\" FROM {cow_table_name}{selection_clause}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DELETE FROM t ORDER BY t.id DESC LIMIT 1 fails on CoW because the query now reads from __cow_target_* and t.id no longer resolves. I hit the same error with UPDATE. Can we preserve the target table qualifier when querying the temp table?

);

let query_sql = format!("SELECT {select_clause} FROM {table_ref}{where_clause}");
let query_sql = format!("SELECT {select_clause} FROM {table_ref}{selection_clause}");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With IDs 1 and 2, UPDATE t SET id = 12 / (2 - id) ORDER BY id LIMIT 1 fails with divide by zero on both paths. Only row 1 is selected, but SET is also evaluated for row 2. Moving the SET projection after ORDER BY and LIMIT would fix this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] DataFusion UPDATE and DELETE can affect more rows than LIMIT allows

2 participants