[CALCITE-7510] EnumerableTableModify UPDATE, DELETE, INSERT support and bug fixes by wasabii · Pull Request #4922 · apache/calcite

wasabii · 2026-05-07T18:38:31Z

Jira Link

Changes Proposed

Added UPDATE support to EnumerableTableModify. Generates code that calls ExtendedEnumerable.update, which is new, and scans the destination list to apply updates.

xiedeyantu · 2026-05-07T23:01:33Z

Thank you for your contribution! Please refer to point four of the PR template and make adjustments accordingly.

4. CRITICAL CONSISTENCY RULE
   The following three items MUST match exactly in wording and meaning:
   (A) The Jira Issue Title
   (B) This Pull Request Title
   (C) Your Git Commit Message

   Format: [CALCITE-XXXX] <Description>
   Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE

   Guidelines for a good Title:
   - Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
   - Focus on the specification and user experience, not the implementation.
   - Make it clear whether it is a bug or a feature.
   - Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").

sonarqubecloud · 2026-05-09T16:02:21Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mihaibudiu · 2026-05-22T03:41:42Z

+
+  /** Generates code for an UPDATE statement.
+   *
+   * <p>The child produces, for each row matched by the WHERE clause, a row of


is "the child" childExp?
This phrase is awkward with lots of commas, you can rearrange it a bit.

mihaibudiu · 2026-05-22T03:42:08Z

+   * updateColumnList.size()} fields are the new values, one per column named in
+   * the SET clause. Filtering by WHERE has already been applied upstream, so
+   * every source row corresponds to an existing row in the modifiable collection
+   * and can be located by full-row content equality.


What happens if the collection is a multiset?
Equality will match multiple rows.
I hope there is a test for this case.

It would match multiple rows. I thought about this, but I am not sure how better to go about it currently. There isn't really any sort of 'row id' passed around between the two sides of this. I also don't really like the full equality test. But, no PK metadata either.

I figured that this was largely good enough for the ServerDdlExecutor.

Actually, I could do something like take the input update list, and ensure that we only ever update one item in the target collection per item in the update list. It's just a lot of complicated accounting, and I don't know if it matter. It certainly doesn't for my use case for this (which is just minimal UPDATE functionality so I can use ServerDdlExecutor as the backend for some other projects test cases.

Calcite is a library with many users, we have to build APIs that have well-defined contracts.
The contract has to be defined clearly, and to make sense. Perhaps it's fine to not support deletion from multi-sets, but this has to be documented and perhaps justified. One justification is that using the DELETE statement you can only delete all or none of the copies of a value from a multiset.

I implemented it as I suggested. With tests. Since all the records in this hypothetical are the same, it just deletes the first record it hits for each item in the source set. So whatever copies remain remain.

mihaibudiu · 2026-05-22T03:45:36Z

+
+  /** Generates code for a DELETE statement.
+   *
+   * <p>{@code Object[].equals()} is reference equality, so


This comment is about the implementation and not what this function does. I would make it a regular comment. The JavaDoc can use more information, though.

mihaibudiu · 2026-05-22T03:45:47Z

+
+  /** Generates code for an INSERT statement.
+   *
+   * <p>When the child row type differs from the table row type (the common case


same comment as above

mihaibudiu · 2026-05-22T03:46:48Z

+  /**
+   * Removes from {@code sink} every row that appears in {@code source}.
+   *
+   * <p>For multi-column rows the backing collection stores {@code Object[]}


another implementation comment

mihaibudiu · 2026-05-22T03:48:41Z

+    for (Object row : tempList) {
+      toRemove.add(Arrays.asList((Object[]) row));
+    }
+    ((Collection<Object[]>) sink).removeIf(row -> toRemove.contains(Arrays.asList(row)));


This seems to rely on the fact that all equal rows will either be deleted or not.
Maybe this is fine, but then function definition should specify this.
There are legit cases where you may want to remove only some of the copies (e.g., IVM).

…nd bug fixes

mihaibudiu

Please do not amend commits during the review process, especially for such a large PR.
It makes it difficult for reviewers to see what has changed

mihaibudiu

I think this is fine, but we won't merge it before the next release, which will hopefully happen soon.

mihaibudiu · 2026-05-26T21:38:37Z

+  }
+
+  /**
+   * Removes from {@code sink} one occurrence per occurrence in {@code source}


this comment is a bit awkward to read

mihaibudiu · 2026-05-26T21:39:26Z

+        sourceRows.add(e.current());
+      }
+    }
+    // Drain source first, then mutate sink, to avoid iterator interference


I would say "mutate sink after having drained the source".
The way the comment is written it makes it look like the next loop also mutates the source

wasabii · 2026-05-26T22:05:43Z

Please do not amend commits during the review process, especially for such a large PR.
It makes it difficult for reviewers to see what has changed

I did this because the policy said The commit text should match the PR title. I assumed you wanted then squashed.

It was super annoying to do and I would have rather not but I thought it was correct.

mihaibudiu · 2026-05-26T22:06:39Z

We'll squash before merging

wasabii force-pushed the upstream-main branch from 48db875 to ba84098 Compare May 9, 2026 14:39

wasabii changed the title ~~[CALCITE-7510]: Implement UPDATE on EnumerableTableModify~~ [CALCITE-7510] EnumerableTableModify UPDATE support May 9, 2026

wasabii force-pushed the upstream-main branch 3 times, most recently from 0f0eb88 to 09685a2 Compare May 9, 2026 15:35

wasabii force-pushed the upstream-main branch from 09685a2 to 740911b Compare May 21, 2026 19:24

wasabii changed the title ~~[CALCITE-7510] EnumerableTableModify UPDATE support~~ [CALCITE-7510] EnumerableTableModify UPDATE, DELETE, INSERT support and bug fixes May 21, 2026

wasabii force-pushed the upstream-main branch from 740911b to 683d8cb Compare May 21, 2026 23:11

mihaibudiu reviewed May 22, 2026

View reviewed changes

wasabii force-pushed the upstream-main branch from 683d8cb to b4c9210 Compare May 26, 2026 00:30

[CALCITE-7510] EnumerableTableModify UPDATE, DELETE, INSERT support a…

6af4079

…nd bug fixes

wasabii force-pushed the upstream-main branch from b4c9210 to 6af4079 Compare May 26, 2026 00:39

mihaibudiu reviewed May 26, 2026

View reviewed changes

mihaibudiu approved these changes May 26, 2026

View reviewed changes

mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label May 26, 2026

Conversation

wasabii commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira Link

Changes Proposed

Uh oh!

xiedeyantu commented May 7, 2026

Uh oh!

sonarqubecloud Bot commented May 9, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wasabii May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wasabii commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaibudiu commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wasabii commented May 7, 2026 •

edited

Loading

wasabii May 26, 2026 •

edited

Loading

wasabii commented May 26, 2026 •

edited

Loading