Skip to content
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

[Spark] Support show tblproperties and update catalog for clustered table #3271

Merged

Conversation

zedtang
Copy link
Collaborator

@zedtang zedtang commented Jun 14, 2024

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

Support show tblproperties for clustered table, and support updating the clustering column properties in the catalog. Remove table properties from describe detail's output since that's using the properties from metadata.

How was this patch tested?

Add verification for table properties, describe detail, and catalog table in verifyClusteringColumns.

Does this PR introduce any user-facing changes?

No

@zedtang zedtang force-pushed the clustered-table-update-catalog branch 2 times, most recently from 293547e to 795a95e Compare June 14, 2024 05:24
@zedtang zedtang force-pushed the clustered-table-update-catalog branch from 795a95e to 4e65a35 Compare June 14, 2024 05:25
Copy link
Contributor

@dabao521 dabao521 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment on lines +48 to +50
// This is necessary because the [[SparkThreadLocalForwardingThreadPoolExecutor]]
// retains a reference to the SparkContext. Without resetting, the new test suite would
// reuse the same SparkContext from the previous suite, despite it being stopped.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also point out that this is mainly for the UpdateCatalogHook background thread to have a new SparkContext to avoid access-after-stop issue between suites?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, added a comment.

@zedtang zedtang force-pushed the clustered-table-update-catalog branch from 46f82cc to 8fc040c Compare June 14, 2024 16:28
@zedtang zedtang force-pushed the clustered-table-update-catalog branch from 5aee0cd to 97c1ef2 Compare June 15, 2024 00:27
@zedtang zedtang force-pushed the clustered-table-update-catalog branch from c4bc884 to 2b5f432 Compare June 15, 2024 06:30
@allisonport-db allisonport-db merged commit 75c6acb into delta-io:master Jun 17, 2024
10 checks passed
@zedtang zedtang deleted the clustered-table-update-catalog branch June 17, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants