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

add supporting remove partitions if delta was partitioned #390

Closed
wants to merge 2 commits into from
Closed

add supporting remove partitions if delta was partitioned #390

wants to merge 2 commits into from

Conversation

hleb-lizunkou1
Copy link
Contributor

It is PR for issue with removing partitions in delta table.
It was discussed here https://delta-users.slack.com/archives/CJ70UCSHM/p1587048581235000
And i implemened proposed solution https://delta-users.slack.com/archives/CJ70UCSHM/p1587068793244400

@@ -79,8 +79,7 @@ trait ImplicitMetadataOperation extends DeltaLogging {
def isNewSchema: Boolean = txn.metadata.schema != mergedSchema
// We need to make sure that the partitioning order and naming is consistent
// if provided. Otherwise we follow existing partitioning
def isNewPartitioning: Boolean = normalizedPartitionCols.nonEmpty &&
txn.metadata.partitionColumns != normalizedPartitionCols
def isNewPartitioning: Boolean = txn.metadata.partitionColumns != normalizedPartitionCols
Copy link
Collaborator

Choose a reason for hiding this comment

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

you need a more intricate condition here:

  1. normalizedPartitionCols.nonEmpty && txn.metadata.partitionColumns != normalizedPartitionCols OR
  2. overwriteSchema && txn.metadata.partitionColumns != normalizedPartitionCols

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i indroduced new variable for it

.format("delta")
.partitionBy("by4")
.save(tempDir.toString)

Copy link
Collaborator

Choose a reason for hiding this comment

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

can you also add a check that the partitioning column indeed did exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check was added

Copy link
Collaborator

@brkyvz brkyvz left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @KevinKarlBob !

@tdas tdas closed this Apr 22, 2020
tdas pushed a commit that referenced this pull request Apr 22, 2020
…rtitioned

It is PR for issue with removing partitions in delta table.
It was discussed here https://delta-users.slack.com/archives/CJ70UCSHM/p1587048581235000
And i implemened proposed solution https://delta-users.slack.com/archives/CJ70UCSHM/p1587068793244400

Closes #390

Co-authored-by: hleb.lizunkou <hleb.lizunkoui@coxautoinc.com>
Signed-off-by: Burak Yavuz <brkyvzgmail.com>
GitOrigin-RevId: eee5573959bc9827f1d381133cde45685f8dbee4
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.

2 participants