Skip to content

Fix: Group transaction changes by table#3360

Open
singhpk234 wants to merge 3 commits intoapache:mainfrom
singhpk234:feature/optimize-transaction
Open

Fix: Group transaction changes by table#3360
singhpk234 wants to merge 3 commits intoapache:mainfrom
singhpk234:feature/optimize-transaction

Conversation

@singhpk234
Copy link
Contributor

@singhpk234 singhpk234 commented Jan 6, 2026

Problem

. Groups ALL changes by table- even if they appear randomly in the input like [A1, B1, A2, C1, A3] → groups to {A:
[A1, A2, A3], B: [B1], C: [C1]}
2. For each table, processes changes sequentially :
- Validate R1 against base metadata → Apply U1 → update currentMetadata
- Validate R2 against updated metadata → Apply U2 → update currentMetadata
- Validate R3 against updated metadata → Apply U3 → update currentMetadata
3. Single commit per table prevents duplicate entity IDs in pendingUpdates

This ensures:

  • Each change's requirements validate against the evolved state
  • Updates are applied in order within each table
  • Only one commit per table (solves the original problem)
  • Conflicts detected early (e.g., second change expecting schema ID 0 fails when it's already 1)

related discussion: #3352 (comment)

Checklist

  • 🛡️ Don't disclose security issues! (contact [email protected])
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

@github-project-automation github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jan 6, 2026
@singhpk234 singhpk234 changed the title Fix: Group transaction changes by table in a transaction Fix: Group transaction changes by table Jan 6, 2026
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/optimize-transaction branch 2 times, most recently from 2380846 to e0f05c0 Compare January 6, 2026 02:53
@dimas-b
Copy link
Contributor

dimas-b commented Jan 6, 2026

nit: Only one commit per table (solves the original problem) - I'd simply say Only one commit per table. This will end up in the git commit log, where the "original problem" might be hard to understand later on... It's also fine to give a summary of the problem instead of the "original" reference if you prefer.

dimas-b
dimas-b previously approved these changes Jan 6, 2026
Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Nice fix. Thanks, @singhpk234 !

Just a couple of minor comments :)

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 6, 2026
Copy link
Contributor

@adnanhemani adnanhemani left a comment

Choose a reason for hiding this comment

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

Awesome change!

@github-actions
Copy link

github-actions bot commented Feb 9, 2026

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Feb 9, 2026
@singhpk234 singhpk234 removed the stale label Feb 9, 2026
@sfc-gh-prsingh sfc-gh-prsingh dismissed stale reviews from dennishuo and dimas-b via 8f1fe58 February 26, 2026 06:53
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/optimize-transaction branch from e0f05c0 to 8f1fe58 Compare February 26, 2026 06:53
@singhpk234
Copy link
Contributor Author

Apologies it took me a while to get back to it, was super swamped with internal stuff.
Addressed @dennishuo feedback to add detailed comment.
Addressed @flyrain's offline feedback to add more tests to iron this.

Please have another pass when you all get some time, really apprecaite your feedbacks here !

@singhpk234 singhpk234 added this to the 1.5.0 milestone Feb 26, 2026
@dimas-b
Copy link
Contributor

dimas-b commented Feb 26, 2026

@singhpk234 : thanks for pushing this forward 👍 Please fix fresh IcebergCatalogHandler conflicts 😅

@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/optimize-transaction branch from 8f1fe58 to 90296e0 Compare February 26, 2026 17:43
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.

6 participants