Skip to content

Use qualified names on DELETE selections #16033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nuno-faria
Copy link
Contributor

Which issue does this PR close?

  • N/A.

Rationale for this change

The logical plan for Deletions was not using qualified column names for the selections, which meant they were not being pushed down to the table. Conversely, Updates were already using qualified names, so this was not a problem for them.

What changes are included in this PR?

  • Added qualified column names to the selections when processing a DELETE.
  • Updated integration tests.
  • Added delete.slt since there were no sqllogictests for the DELETE operator (follows a similar structure to update.slt).

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions github-actions bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels May 12, 2025
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @nuno-faria this is good to have although I'm afraid for now UPDATE/DEL DML are not yet supported

Btw @alamb this is prob a good question is there a place in roadmap to support UPDATE/DELETE DMLs? for a single machine engine it might be beneficial. DuckDB supports it

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @nuno-faria -- this PR makes sense to me -- the improved test coverage is especially nice 👌

@alamb
Copy link
Contributor

alamb commented May 16, 2025

Btw @alamb this is prob a good question is there a place in roadmap to support UPDATE/DELETE DDLs? for a single machine engine it might be beneficial. DuckDB supports it

TLDR is I don't think there is any standard way to support UPDATE/DELETE in any of the formats that DataFusion comes with(parquet, json, avro, jason)

My understanding is that the DML planning support in Datafusion is used for:

  1. Formats (e.g. iceberg) that do have support for update/delete
  2. Systems (e.g. some proprietary db systems) that can support update/delete

This is something we should probably add to the DataFusion documentation somewhere 🤔

@comphead
Copy link
Contributor

Agree, I was mostly thinking about in memory dataset or Iceberg as it also supports DML, lets wait for the real use case to come

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants