Skip to content

Spark: Add ExpireSnapshotsProcedure#1874

Merged
rdblue merged 4 commits intoapache:masterfrom
aokolnychyi:expire-snapshots-procedure
Dec 4, 2020
Merged

Spark: Add ExpireSnapshotsProcedure#1874
rdblue merged 4 commits intoapache:masterfrom
aokolnychyi:expire-snapshots-procedure

Conversation

@aokolnychyi
Copy link
Contributor

This PR adds a stored procedure to expire snapshots.

The main author of this change is @liukun4515. I took commits from #1819, rebased, and added more changes to match the current state of procedures.

Resolves #1597.

@aokolnychyi aokolnychyi changed the title Expire snapshots procedure Spark: Add ExpireSnapshotsProcedure Dec 4, 2020
@aokolnychyi
Copy link
Contributor Author

@github-actions github-actions bot added the spark label Dec 4, 2020

@Test
public void testInvalidExpireSnapshotsCases() {
AssertHelpers.assertThrows("Should not allow mixed args",
Copy link
Member

Choose a reason for hiding this comment

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

Not sure we need some of the more generic checks here, like testing if named and pos args cannot be mixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a bit weird but we have such checks in other procedures so I added for consistency.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds fine, It doesn't really matter since they run so quickly.

ExpireSnapshotsAction action = actions.expireSnapshots();

if (olderThanMillis != null) {
action.expireOlderThan(olderThanMillis);
Copy link
Member

Choose a reason for hiding this comment

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

So we are going with the action default expire time? Just checking because I thougt some folks wanted that changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The action delegates to ExpireSnapshots table API that, in turn, respects the default table props we added recently.

Copy link
Member

Choose a reason for hiding this comment

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

Ah! Excellent, I forgot about that

@rdblue rdblue merged commit b5f08d2 into apache:master Dec 4, 2020
pvary pushed a commit to pvary/iceberg that referenced this pull request Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spark SQL Extensions: Expire snapshots

4 participants