You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/05/18 02:10:47 UTC
[GitHub] [hudi] gnailJC opened a new pull request, #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
gnailJC opened a new pull request, #5616:
URL: https://github.com/apache/hudi/pull/5616
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contribute/how-to-contribute before opening a pull request.*
## What is the purpose of the pull request
HoodieSqlCommonUtils.withSparkConf() determines whether it is a hudi config key by judging the prefix `hoodie.`, which will cause the `as.of.instant` configuration to be handled incorrectly. This PR is compatible with the judgment logic of `as.of.instant`.
## Brief change log
*(for example:)*
- *Modify AnnotationLocation checkstyle rule in checkstyle.xml*
## 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:)*
- *Added integration tests for end-to-end.*
- *Added HoodieClientWriteTest to verify the change.*
- *Manually verified the change by running a job locally.*
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] alexeykudinkin commented on a diff in pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
alexeykudinkin commented on code in PR #5616:
URL: https://github.com/apache/hudi/pull/5616#discussion_r982612084
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieCommonConfig.java:
##########
@@ -36,6 +36,11 @@ public class HoodieCommonConfig extends HoodieConfig {
.defaultValue(false)
.withDocumentation("Enables support for Schema Evolution feature");
+ public static final ConfigProperty<String> TIMESTAMP_AS_OF = ConfigProperty
+ .key("as.of.instant")
+ .noDefaultValue()
+ .withDocumentation("The query instant for time travel. Without specified this option, we query the latest snapshot.");
Review Comment:
nit: Let's fix the wording while we're at it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] alexeykudinkin commented on pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
alexeykudinkin commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1260027941
@gnailJC can you please elaborate if this is addressing an issue that you've faced or is it just code cleanup?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
xushiyan commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1260393874
> @gnailJC can you please elaborate if this is addressing an issue that you've faced or is it just code cleanup?
@alexeykudinkin this is reported in https://github.com/apache/hudi/issues/5586
This is not really a blocker; more of a usability with time travel, which can be mitigated as suggested in https://github.com/apache/hudi/issues/5586#issuecomment-1128319091
I updated the PR so that we can keep the compatibility for using this and filed https://issues.apache.org/jira/browse/HUDI-4935 for breaking change in 1.0.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1129576985
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1129500566
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a diff in pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
xushiyan commented on code in PR #5616:
URL: https://github.com/apache/hudi/pull/5616#discussion_r878681998
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala:
##########
@@ -780,6 +780,9 @@ object DataSourceOptionsHelper {
) ++ translateConfigurations(parameters)
}
+ def isHoodieConfigKey(key: String): Boolean =
+ key.startsWith("hoodie.") || key == DataSourceReadOptions.TIME_TRAVEL_AS_OF_INSTANT.key
+
Review Comment:
we should avoid this kind of special handling. this is a typical code smell.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] gnailJC commented on a diff in pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
gnailJC commented on code in PR #5616:
URL: https://github.com/apache/hudi/pull/5616#discussion_r875489218
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -250,7 +250,7 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
(baseConfig: Map[String, String] = Map.empty): Map[String, String] = {
baseConfig ++ DFSPropertiesConfiguration.getGlobalProps.asScala ++ // Table options has the highest priority
(spark.sessionState.conf.getAllConfs ++ HoodieOptionConfig.mappingSqlOptionToHoodieParam(options))
- .filterKeys(_.startsWith("hoodie."))
+ .filterKeys(DataSourceOptionsHelper.isHoodieConfigKey)
Review Comment:
> The logic has changed ?
Yes, if only use `startWith(''hoodie.'')` to determine whether it is the hoodie config key, then "as.of.instant" will be misjudged and cannot achieve the expected purpose.
for example:
create a hoodie_tbl table by sql:
`
CREATE external TABLE hoodie_tbl
using hudi
options (
hoodie.datasource.query.type='snapshot',
as.of.instant='20220512165111404'
) LOCATION 'xxx';
`
hoodie_tbl will always query the latest snapshot, not historical snapshots(20220512165111404), because `as.of.instant `will be filtered and will not take effect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1129498948
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] alexeykudinkin commented on a diff in pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
alexeykudinkin commented on code in PR #5616:
URL: https://github.com/apache/hudi/pull/5616#discussion_r981695841
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala:
##########
@@ -780,6 +780,9 @@ object DataSourceOptionsHelper {
) ++ translateConfigurations(parameters)
}
+ def isHoodieConfigKey(key: String): Boolean =
+ key.startsWith("hoodie.") || key == DataSourceReadOptions.TIME_TRAVEL_AS_OF_INSTANT.key
+
Review Comment:
Agree w/ you @xushiyan, but as far as i understand has already been exposed as it is (`as.of.instant`) and as such we will have to go t/h migration to change it.
I think that decision we can take separately from fixing an issue of not propagating this config.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala:
##########
@@ -780,6 +780,9 @@ object DataSourceOptionsHelper {
) ++ translateConfigurations(parameters)
}
+ def isHoodieConfigKey(key: String): Boolean =
+ key.startsWith("hoodie.") || key == DataSourceReadOptions.TIME_TRAVEL_AS_OF_INSTANT.key
+
Review Comment:
@gnailJC let's actually reframe it like following (to make it more easily extensible):
val hudiConfigKeyPrefixes = Seq("hoodie.", "as.of.instant")
def isHoodieConfigKey(key): = hudiConfigKeyPrefixes.exists(prefix => key.startsWith(prefix))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1260415431
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
}, {
"hash" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11825",
"triggerID" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726)
* c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11825)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #5616: [MINOR]: Optimize the judgment logic of `SparkDataSourceOptions Key`
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #5616:
URL: https://github.com/apache/hudi/pull/5616#discussion_r875415160
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -250,7 +250,7 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
(baseConfig: Map[String, String] = Map.empty): Map[String, String] = {
baseConfig ++ DFSPropertiesConfiguration.getGlobalProps.asScala ++ // Table options has the highest priority
(spark.sessionState.conf.getAllConfs ++ HoodieOptionConfig.mappingSqlOptionToHoodieParam(options))
- .filterKeys(_.startsWith("hoodie."))
+ .filterKeys(DataSourceOptionsHelper.isHoodieConfigKey)
Review Comment:
The logic has changed ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1260783687
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
}, {
"hash" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11825",
"triggerID" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11825)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5616:
URL: https://github.com/apache/hudi/pull/5616#issuecomment-1260412074
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726",
"triggerID" : "beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b",
"triggerType" : "PUSH"
}, {
"hash" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* beb60e0e8dd7b36d2fdc09c234eac5ea7f5a830b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8726)
* c1811c42ef34dfbd4a80ee6e55a9e11de5a4deaa UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan merged pull request #5616: [HUDI-4936] Fix `as.of.instant` not recognized as hoodie config
Posted by GitBox <gi...@apache.org>.
xushiyan merged PR #5616:
URL: https://github.com/apache/hudi/pull/5616
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org