[HUDI-3780] improve drop partitions #5178
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
XuQianJin-Stars
force-pushed
the
fix-delete-partitions
branch
from
ae1d66b to
8889b34
Compare
XuQianJin-Stars
changed the title
XuQianJin-Stars
changed the title
XuQianJin-Stars
force-pushed
the
fix-delete-partitions
branch
from
8889b34 to
ee4501b
Compare
Contributor
Author
|
hi @vinothchandar @xushiyan this pr fix for #4291, please hive a look. |
xiarixiaoyao
reviewed
Apr 3, 2022
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
| import javax.annotation.Nonnull; | ||
| import org.apache.avro.AvroTypeException; |
Contributor
There was a problem hiding this comment.
why change import order？
| } catch (Exception e) { | ||
| throw new HoodieException("Failed to get commit metadata", e); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
why move this method from TableSchemaResolver to this class
Contributor
Author
There was a problem hiding this comment.
why move this method from TableSchemaResolver to this class
@vinothchandar suggest extract this out to a separate static helper.
|
|
||
| @Override | ||
| public void dropPartitions(String tableName, List<String> partitionsToDrop) { | ||
| throw new UnsupportedOperationException("No support for dropPartitions yet."); |
Contributor
There was a problem hiding this comment.
pls do not delete origin code annotation
// bigQuery discovers the new partitions automatically, so do nothing.
| import org.apache.spark.sql.types.StructType | ||
|
|
||
| import java.util | ||
| import scala.collection.JavaConverters.propertiesAsScalaMapConverter |
Contributor
There was a problem hiding this comment.
java import and scala import should be separated
|
|
||
| val options = hoodieCatalogTable.catalogProperties ++ tableConfig.getProps.asScala.toMap ++ extraOptions | ||
| val options: Map[String, String] = hoodieCatalogTable.catalogProperties ++ tableConfig.getProps.asScala.toMap ++ sparkSession.sqlContext.conf.getAllConfs ++ extraOptions | ||
| val hiveSyncConfig = buildHiveSyncConfig(options, hoodieCatalogTable) |
Contributor
There was a problem hiding this comment.
it may be good to reuse code？ i see double defined
| val tableConfig = hoodieCatalogTable.tableConfig | ||
| val tableSchema = hoodieCatalogTable.tableSchema | ||
| val partitionColumns = tableConfig.getPartitionFieldProp.split(",").map(_.toLowerCase) | ||
| val partitionSchema = StructType(tableSchema.filter(f => partitionColumns.contains(f.name))) |
Contributor
There was a problem hiding this comment.
.toLowerCase(Locale.ROOT)
| val partitionSchema = StructType(tableSchema.filter(f => partitionColumns.contains(f.name))) | ||
|
|
||
| assert(hoodieCatalogTable.primaryKeys.nonEmpty, | ||
| s"There are no primary key defined in table ${hoodieCatalogTable.table.identifier}, cannot execute delete operator") |
| PARTITIONPATH_FIELD.key -> tableConfig.getPartitionFieldProp, | ||
| HiveSyncConfig.HIVE_SYNC_MODE.key -> HiveSyncMode.HMS.name(), | ||
| HiveSyncConfig.HIVE_SUPPORT_TIMESTAMP_TYPE.key -> "true", | ||
| HoodieWriteConfig.DELETE_PARALLELISM_VALUE.key -> "200", |
Contributor
There was a problem hiding this comment.
why 200？ it will be better to make it Configurable
xiarixiaoyao
requested review from
xiarixiaoyao
and removed request for
xiarixiaoyao
XuQianJin-Stars
force-pushed
the
fix-delete-partitions
branch
from
ee4501b to
d77964d
Compare
Collaborator
xiarixiaoyao
approved these changes
Apr 4, 2022
Contributor
|
LGTM |
4 tasks
alexeykudinkin
added a commit
that referenced
this pull request
Jan 20, 2023
…r CTAS and other commands (#7607) ### Change Logs While following up and adding support for BrooklynData Benchmarks we've discovered that CTAS isn't properly propagating configs due to a recent change in [#5178](https://github.com/apache/hudi/pull/5178/files#diff-560283e494c8ba8da102fc217a2201220dd4db731ec23d80884e0f001a7cc0bcR117) Unfortunately logic of handling the configuration in `ProvidesHoodieConfig` become overly complicated and fragmented. This PR takes a stab at it trying to unify and streamline fusing the options from different sources (Spark Catalog props, Table properties, Spark SQL conf, overrides, etc) making sure different Spark SQL operations do handle it in much the same way (for ex, `MERGE INTO`, CTAS, `INSERT INTO`, etc) Changes - Simplify and unify `ProvidesHoodieConfig` configuration fusion from different sources - Fixing CTAS to override "hoodie.combine.before.insert" as "false"
fengjian428
pushed a commit
to fengjian428/hudi
that referenced
this pull request
Jan 31, 2023
…r CTAS and other commands (apache#7607) ### Change Logs While following up and adding support for BrooklynData Benchmarks we've discovered that CTAS isn't properly propagating configs due to a recent change in [apache#5178](https://github.com/apache/hudi/pull/5178/files#diff-560283e494c8ba8da102fc217a2201220dd4db731ec23d80884e0f001a7cc0bcR117) Unfortunately logic of handling the configuration in `ProvidesHoodieConfig` become overly complicated and fragmented. This PR takes a stab at it trying to unify and streamline fusing the options from different sources (Spark Catalog props, Table properties, Spark SQL conf, overrides, etc) making sure different Spark SQL operations do handle it in much the same way (for ex, `MERGE INTO`, CTAS, `INSERT INTO`, etc) Changes - Simplify and unify `ProvidesHoodieConfig` configuration fusion from different sources - Fixing CTAS to override "hoodie.combine.before.insert" as "false"
nsivabalan
pushed a commit
to nsivabalan/hudi
that referenced
this pull request
Mar 22, 2023
…r CTAS and other commands (apache#7607) While following up and adding support for BrooklynData Benchmarks we've discovered that CTAS isn't properly propagating configs due to a recent change in [apache#5178](https://github.com/apache/hudi/pull/5178/files#diff-560283e494c8ba8da102fc217a2201220dd4db731ec23d80884e0f001a7cc0bcR117) Unfortunately logic of handling the configuration in `ProvidesHoodieConfig` become overly complicated and fragmented. This PR takes a stab at it trying to unify and streamline fusing the options from different sources (Spark Catalog props, Table properties, Spark SQL conf, overrides, etc) making sure different Spark SQL operations do handle it in much the same way (for ex, `MERGE INTO`, CTAS, `INSERT INTO`, etc) Changes - Simplify and unify `ProvidesHoodieConfig` configuration fusion from different sources - Fixing CTAS to override "hoodie.combine.before.insert" as "false"
fengjian428
pushed a commit
to fengjian428/hudi
that referenced
this pull request
Apr 5, 2023
…r CTAS and other commands (apache#7607) ### Change Logs While following up and adding support for BrooklynData Benchmarks we've discovered that CTAS isn't properly propagating configs due to a recent change in [apache#5178](https://github.com/apache/hudi/pull/5178/files#diff-560283e494c8ba8da102fc217a2201220dd4db731ec23d80884e0f001a7cc0bcR117) Unfortunately logic of handling the configuration in `ProvidesHoodieConfig` become overly complicated and fragmented. This PR takes a stab at it trying to unify and streamline fusing the options from different sources (Spark Catalog props, Table properties, Spark SQL conf, overrides, etc) making sure different Spark SQL operations do handle it in much the same way (for ex, `MERGE INTO`, CTAS, `INSERT INTO`, etc) Changes - Simplify and unify `ProvidesHoodieConfig` configuration fusion from different sources - Fixing CTAS to override "hoodie.combine.before.insert" as "false"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tips
What is the purpose of the pull request
(For example: This pull request adds quick-start document.)
Brief change log
(for example:)
Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.