Skip to content

Comments

[HUDI-6236] write hive_style_partitioning_enable to table config in D…#8752

Merged
danny0405 merged 1 commit intoapache:masterfrom
voonhous:HUDI-6236
May 20, 2023
Merged

[HUDI-6236] write hive_style_partitioning_enable to table config in D…#8752
danny0405 merged 1 commit intoapache:masterfrom
voonhous:HUDI-6236

Conversation

@voonhous
Copy link
Member

…eltaStreamer entrypoints

Change Logs

In Hudi 0.10.X, Hudi assumes that hoodie table properties contains the hoodie.datasource.write.hive_style_partitioning configuration value.

As such hoodie.properties should contain this config for all Hudi tables created by Hudi > 0.10.X. However, in DeltaStreamer entrypoints, this configuration is not written to hoodie.properties, which will cause incorrect query results via Spark.

This PR fixes the aforementioned assumptions by ensuring that hoodie.datasource.write.hive_style_partitioning is written to hoodie.properties.

Impact

Describe any public API or user-facing feature change or any performance impact.

None

Risk level (write none, low medium or high below)

If medium or high, explain what verification was done to mitigate the risks.

Low

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

  • The config description must be updated if new configs are added or the default value of the configs are changed
  • Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
    ticket number here and follow the instruction to make
    changes to the website.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@voonhous
Copy link
Member Author

@boneanxs for visibility

Copy link
Contributor

@boneanxs boneanxs left a comment

Choose a reason for hiding this comment

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

+1 for this

@voonhous
Copy link
Member Author

Also, we may need add an improvement for how tables are initialised in the future so that all entrypoints will use a common HoodieTableMetaClient initialiser.

Instead of the current implementation where all entrypoints are dictating their own means of creating a HoodieTableMetaClient.

Once that is done, we will also need a configuration validation component to check the configs of a table.

@TengHuo
Copy link
Contributor

TengHuo commented May 18, 2023

According the the code in HoodieSparkSqlWriter, there are few more config when building HoodieTableConfig.

https://github.com/apache/hudi/blob/720c9d76c1d281b7a1305a205d195a4f179ce36a/hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala#LL203C13-L203C13

I think it's better we can align DeltaSync according to HoodieSparkSqlWriter.

BTW: totally agree, we should use a unique entrypoint for this HoodieTableConfig builder.

@voonhous
Copy link
Member Author

According the the code in HoodieSparkSqlWriter, there are few more config when building HoodieTableConfig.

https://github.com/apache/hudi/blob/720c9d76c1d281b7a1305a205d195a4f179ce36a/hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala#LL203C13-L203C13

I think it's better we can align DeltaSync according to HoodieSparkSqlWriter.

BTW: totally agree, we should use a unique entrypoint for this HoodieTableConfig builder.

Missing configs that are in HoodieSparkSqlWriter.scala, but not in DeltaSync.java.

In initializeEmptyTable()

.setCommitTimezone()
.setDatabaseName()
.set(timestampKeyGeneratorConfigs)

In readFromSource()

.setCDCEnabled()
.setCDCSupplementalLoggingMode()
.setCommitTimezone()
.setDatabaseName()
.setPreCombineField()
.setRecordKeyFields()
.set(timestampKeyGeneratorConfigs)

@danny0405 danny0405 self-assigned this May 18, 2023
@danny0405 danny0405 added the component:catalog-sync Catalog-sync related label May 18, 2023
@voonhous
Copy link
Member Author

Apologies again.

Fixed the checkstyle violations and re-executed a mvn clean install on my end.

@voonhous
Copy link
Member Author

@hudi-bot run azure

@hudi-bot
Copy link
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@danny0405 danny0405 closed this May 19, 2023
@danny0405 danny0405 reopened this May 19, 2023
@danny0405 danny0405 merged commit bcfd354 into apache:master May 20, 2023
@voonhous voonhous deleted the HUDI-6236 branch December 7, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:catalog-sync Catalog-sync related

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants