-
Notifications
You must be signed in to change notification settings - Fork 923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[KYUUBI #5407][AUTHZ] Tests for Iceberg system procedures of snapshot management #5394
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5394 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 590 588 -2
Lines 33493 33427 -66
Branches 4424 4393 -31
======================================
+ Misses 33493 33427 -66 see 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
It looks like the failed UT is unstable |
Some flaky tests may fail occasionally. No worries. |
}) | ||
|
||
val calendar = Calendar.getInstance() | ||
calendar.add(Calendar.DAY_OF_MONTH, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this shifting to 1 month later stand for ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meaning of Iceberg's rollbackToTimestamp command is to roll back the snapshot to the latest snapshot less than calendar
time. In order to ensure that this snapshot must exist, I added for one day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced to use the target snapshot timestamp as the base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks for the continuous contribution from @yabola . |
@bowenliang123 Thank you for your help! |
Why are the changes needed?
To close #5407 .
Follow up for #5248 .
Add some UT for snapshot management procedures. These procedures require alter permissions.
rollback_to_snapshot (https://iceberg.apache.org/docs/latest/spark-procedures/#rollback_to_snapshot):
Usage:
CALL catalog_name.system.rollback_to_snapshot('db.sample', 1)
Meaning: rollback a table to a specific snapshot ID.
rollback_to_timestamp (https://iceberg.apache.org/docs/latest/spark-procedures/#rollback_to_timestamp)
Usage:
CALL catalog_name.system.rollback_to_timestamp('db.sample', TIMESTAMP '2021-06-30 00:00:00')
Meaning: rollback the table to the latest snapshot less than time.
set_current_snapshot (https://iceberg.apache.org/docs/latest/spark-procedures/#set_current_snapshot)
Usage:
CALL catalog_name.system.set_current_snapshot('db.sample', 1)
Meaning: Set a table to a specific snapshot ID.
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request
Was this patch authored or co-authored using generative AI tooling?
No