Skip to content

Conversation

@rdblue
Copy link
Contributor

@rdblue rdblue commented Dec 6, 2020

The TableBuilder makes it easier to use catalogs from Spark, but the CachingCatalog doesn't currently support the builder so tests fail. This implements the builder for the CachingCatalog and uses it for the existing transaction methods. The existing tests should now exercise the builder.

@github-actions github-actions bot added the core label Dec 6, 2020
.withPartitionSpec(spec)
.withLocation(location)
.withProperties(properties)
.create();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should consider moving these to be default implementations in the interface.

@aokolnychyi
Copy link
Contributor

This PR looks good to me but I think we should keep calling the invalidate method in CachingCatalog.

Copy link
Contributor

@aokolnychyi aokolnychyi left a comment

Choose a reason for hiding this comment

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

+1

@rdblue rdblue merged commit 94705c6 into apache:master Dec 7, 2020
@rdblue
Copy link
Contributor Author

rdblue commented Dec 7, 2020

Merged this. Thanks for reviewing, @rymurr and @aokolnychyi!

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.

3 participants