Repository navigation
refactor(csharp): split SEA metadata code by result format - #704
Open
vuanhphung wants to merge 2 commits into
Open
vuanhphung wants to merge 2 commits into
vuanhphung wants to merge 2 commits into
Conversation
Follow-up to #692. Move SEA metadata decoding and result building into StatementExecution/Metadata, with mirrored Native/ and Legacy/ (SHOW) implementations of IMetadataDecoder and IMetadataResultBuilder. MetadataRowReader is the only place that branches on the format and holds the rules both formats share, so statement Get*Async methods no longer branch on IsNative. Behavior is unchanged. Unit tests are split along the same line and add parity tests that decode equivalent native and SHOW fixtures. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
vuanhphung
added
integration-test
Contributor
|
🚀 Integration tests triggered! View workflow run |
1 similar comment
Contributor
|
🚀 Integration tests triggered! View workflow run |
Filter native tables by type before sorting, as on main, and apply the table-type filter in each decoder. Make MetadataRowReader.Decoder private and fail fast if a column row is read before it is numbered. Keep result_count an int, update the design doc, and add tests that pin GetObjects numbering order and the SHOW result builders' columns. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
github-actions
Bot
removed
the
integration-test
Contributor
|
🔒 Integration test approval reset New commits were pushed to this PR. The A maintainer must re-review the changes and re-add the label to trigger tests again. Why is this necessary?
Latest commit: a6c1d60 |
vuanhphung
added
integration-test
Contributor
|
🚀 Integration tests triggered! View workflow run |
1 similar comment
Contributor
|
🚀 Integration tests triggered! View workflow run |
Collaborator
Author
|
@eric-wang-1990 would you mind taking a look at this? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's Changed
Follow-up to #692, implementing the layout proposal from its review. SEA metadata code moves into
StatementExecution/Metadata/, with mirroredNative/andLegacy/(SHOW) implementations ofIMetadataDecoderandIMetadataResultBuilder. Each folder has the same per-operation files, so an operation added to one format only fails the build (primary and foreign keys are decoded only for SHOW; native keys go straight to the builder).MetadataRowReaderis now the only place that branches on the format. It also holds the definitions both formats use:DefaultTableType, the table-type match, SHOW column numbering, and the GetObjects ordinal offset.CatalogFilterbuilds its regex once per filter.The statement's
Get*Asyncmethods no longer branch onIsNative; they pick a builder and return. The SHOW result builders that were inline there, andFlatColumnsResultBuilder, move toLegacy/MetadataResultBuilder. The native primary-key and foreign-key decoders were unused and are removed. Results are unchanged. Theresult_countandresult_tablestrace tags are now also emitted for native results.Comparator run: https://databricks.slack.com/archives/C0B70MDS8DP/p1791872570660359
Unit tests follow the same split: parity tests decode equivalent native and SHOW fixtures and expect identical rows, and format-specific tests cover native catalog re-filtering, Thrift ordering, SHOW parsing, and the SHOW builders' output columns. All 1,175 unit tests pass locally.
This pull request and its description were written by Isaac.