Skip to content

Fix RTAS detection logic in catalog - #699

Open
jiang95-dev wants to merge 2 commits into
linkedin:mainfrom
jiang95-dev:lejiang/rtas-transaction
Open

Fix RTAS detection logic in catalog#699
jiang95-dev wants to merge 2 commits into
linkedin:mainfrom
jiang95-dev:lejiang/rtas-transaction

Conversation

@jiang95-dev

@jiang95-dev jiang95-dev commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Currently TableOps recognize an RTS operation by isPutSnapshot && isMetadataUpdate && base != null. This is wrong since a transaction containing snapshot and metadata update is totally valid without being an RTAS operation. Adding a flag in TableOps and passing it from Catalog to recognize RTAS.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

For all the boxes checked, please include additional details of the changes made in this pull request.

Testing Done

  • Manually Tested on local docker setup. Please include commands ran, and their output.
  • Added new tests for the changes made.
  • Updated existing tests to reflect the changes made.
  • No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
  • Some other form of testing like staging or soak time in production. Please explain.

For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

For all the boxes checked, include additional details of the changes made in this pull request.

// only place where RTAS is unambiguously known: by the time doCommit runs, a replace and an
// ordinary metadata-plus-snapshot transaction look identical. Every Catalog replace entry
// point (including Catalog#newReplaceTableTransaction) funnels through this builder.
((OpenHouseTableOperations) ops).markReplaceTransaction();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we casting? either this takes in OpenhouseTableOperations always or this will result in a bad cast exception and using the generic TableOperations was not useful.

@jiang95-dev jiang95-dev Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes it's better to always use OpenHouseTableOperations in OpenHouseCatalog.

@jiang95-dev
jiang95-dev force-pushed the lejiang/rtas-transaction branch from 07e55df to 95f2b56 Compare August 27, 2026 20:39
}

// ============================= OpenHouse Views (gated, off by default) =============================
// ============================= OpenHouse Views (gated, off by default)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why did this file change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's due to style check.

@jiang95-dev
jiang95-dev force-pushed the lejiang/rtas-transaction branch from a170414 to 88985f8 Compare August 28, 2026 01:05
@jiang95-dev
jiang95-dev force-pushed the lejiang/rtas-transaction branch from 88985f8 to 10b257e Compare August 28, 2026 02:37
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.

2 participants