test: Add test for querying iceberg branch#27268
Merged
agrawalreetika merged 1 commit intoprestodb:masterfrom Mar 6, 2026
Merged
test: Add test for querying iceberg branch#27268agrawalreetika merged 1 commit intoprestodb:masterfrom
agrawalreetika merged 1 commit intoprestodb:masterfrom
Conversation
Contributor
Reviewer's GuideAdds a new Iceberg connector integration test to validate querying table branches via both FOR SYSTEM_VERSION AS OF and dot-notation syntaxes for branches, and touches the Iceberg connector documentation file (no visible content changes in the provided diff). File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider wrapping the table creation and test body in a try/finally (or equivalent cleanup mechanism) so that the test table is reliably dropped even if an assertion fails, preventing state leakage across tests.
- The dot-notation reference "test_branch_dot_notation.branch_audit_branch" encodes a specific naming convention (branch_ + branchName); it would be safer to either derive this from a single constant/utility or add a short comment explaining the expected mapping so future connector changes don’t silently break the test.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider wrapping the table creation and test body in a try/finally (or equivalent cleanup mechanism) so that the test table is reliably dropped even if an assertion fails, preventing state leakage across tests.
- The dot-notation reference "test_branch_dot_notation.branch_audit_branch" encodes a specific naming convention (branch_ + branchName); it would be safer to either derive this from a single constant/utility or add a short comment explaining the expected mapping so future connector changes don’t silently break the test.
## Individual Comments
### Comment 1
<location path="presto-docs/src/main/sphinx/connector/iceberg.rst" line_range="2227" />
<code_context>
Iceberg supports branches and tags which are named references to snapshots.
-Query Iceberg table by specifying the branch name:
+Query Iceberg table by specifying the branch name using ``FOR SYSTEM_VERSION AS OF``:
.. code-block:: sql
</code_context>
<issue_to_address>
**suggestion (typo):** Consider adding an article for grammatical correctness ("Query an Iceberg table").
Suggested wording: `Query an Iceberg table by specifying the branch name using FOR SYSTEM_VERSION AS OF:`
```suggestion
Query an Iceberg table by specifying the branch name using ``FOR SYSTEM_VERSION AS OF``:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Iceberg supports branches and tags which are named references to snapshots. | ||
|
|
||
| Query Iceberg table by specifying the branch name: | ||
| Query Iceberg table by specifying the branch name using ``FOR SYSTEM_VERSION AS OF``: |
Contributor
There was a problem hiding this comment.
suggestion (typo): Consider adding an article for grammatical correctness ("Query an Iceberg table").
Suggested wording: Query an Iceberg table by specifying the branch name using FOR SYSTEM_VERSION AS OF:
Suggested change
| Query Iceberg table by specifying the branch name using ``FOR SYSTEM_VERSION AS OF``: | |
| Query an Iceberg table by specifying the branch name using ``FOR SYSTEM_VERSION AS OF``: |
steveburnett
approved these changes
Mar 5, 2026
Contributor
steveburnett
left a comment
There was a problem hiding this comment.
LGTM! (docs)
Pull branch, local doc build, looks good. Thanks!
garimauttam
pushed a commit
to garimauttam/presto
that referenced
this pull request
Mar 9, 2026
## Description Add test for querying the iceberg branch ## Motivation and Context Add test for querying the iceberg branch ## Impact Add test for querying the iceberg branch ## Test Plan Test Added ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == NO RELEASE NOTE == ``` ## Summary by Sourcery Tests: - Add an Iceberg integration test that verifies querying a branch via FOR SYSTEM_VERSION AS OF and dot-notation syntaxes and compares results with the main table.
This was referenced Mar 31, 2026
15 tasks
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
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.
Description
Add test for querying the iceberg branch
Motivation and Context
Add test for querying the iceberg branch
Impact
Add test for querying the iceberg branch
Test Plan
Test Added
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
Summary by Sourcery
Tests: