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/12/15 22:30:15 UTC
[GitHub] [hudi] hemanth-gowda-12 opened a new pull request, #7474: Added validation for Partition Path to not begin with "/"
hemanth-gowda-12 opened a new pull request, #7474:
URL: https://github.com/apache/hudi/pull/7474
### Change Logs
Added validation for partition path to not start with "/".
### Impact
Found that upserts have the behavior of inserts when partition path starts with "/".
The change is to validate and fail fast in the above mentioned scenario.
### Risk level (write none, low medium or high below)
low
### Documentation Update
N/A
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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] hemanth-gowda-12 commented on a diff in pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hemanth-gowda-12 commented on code in PR #7474:
URL: https://github.com/apache/hudi/pull/7474#discussion_r1051085879
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieKey.java:
##########
@@ -53,6 +53,9 @@ public void setPartitionPath(String partitionPath) {
}
public String getPartitionPath() {
+ if (partitionPath.trim().startsWith("/")) {
Review Comment:
@nsivabalan , I'm using the Java Client and not Spark. I see that all key generators extend directly or indirectly the Built-in key gen [link](https://github.com/apache/hudi/blob/master/hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/BuiltinKeyGenerator.java) which in turn implements SparkKeyGeneratorInterface.
**I'm using the Java Client and I want my implementation simple and without any Spark dependency.**
As for " _probably we should do it elsewhere once._" - I understand the performance impact and is not ideal. Do you have a suggestion as to where else the validation would be more suitable?
--
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] hemanth-gowda-12 commented on a diff in pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hemanth-gowda-12 commented on code in PR #7474:
URL: https://github.com/apache/hudi/pull/7474#discussion_r1051085879
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieKey.java:
##########
@@ -53,6 +53,9 @@ public void setPartitionPath(String partitionPath) {
}
public String getPartitionPath() {
+ if (partitionPath.trim().startsWith("/")) {
Review Comment:
@nsivabalan , I'm using the Java Client and not Spark. I see that all key generators extend directly or indirectly the Build-in key gen [link](https://github.com/apache/hudi/blob/master/hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/BuiltinKeyGenerator.java) which in turn implements SparkKeyGeneratorInterface.
**I'm using the Java Client and I want my implementation simple and without any Spark dependency.**
As for _probably we should do it elsewhere once._ - I understand the performance impact and is not ideal. Do you have a suggestion as to where else the validation would be more suitable?
--
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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1353859489
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9fb6bf2b503384db79feca453fcefa040f79c4b7 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] hudi-bot commented on pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1358052821
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
}, {
"hash" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865",
"triggerID" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9fb6bf2b503384db79feca453fcefa040f79c4b7 Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773)
* bd70e473f8a20f4d631980298ec201aa730713ae Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865)
<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] nsivabalan commented on a diff in pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on code in PR #7474:
URL: https://github.com/apache/hudi/pull/7474#discussion_r1050296097
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieKey.java:
##########
@@ -53,6 +53,9 @@ public void setPartitionPath(String partitionPath) {
}
public String getPartitionPath() {
+ if (partitionPath.trim().startsWith("/")) {
Review Comment:
this might be costly to do it for every record key. probably we should do it elsewhere once.
Also in general, mostly everyone uses out of the box key generators and hence this was not an issue. this might be a problem only when you use your own custom key gen.
Curious to know if none of the built in key generators worked for u?
--
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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1358155535
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
}, {
"hash" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865",
"triggerID" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"triggerType" : "PUSH"
}, {
"hash" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9fb6bf2b503384db79feca453fcefa040f79c4b7 Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773)
* bd70e473f8a20f4d631980298ec201aa730713ae Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865)
* 0cd6ed4601354e8e30757ffc67335b5b1eb0410e 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] hudi-bot commented on pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1358371304
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
}, {
"hash" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865",
"triggerID" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"triggerType" : "PUSH"
}, {
"hash" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13866",
"triggerID" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0cd6ed4601354e8e30757ffc67335b5b1eb0410e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13866)
<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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1358163987
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
}, {
"hash" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865",
"triggerID" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"triggerType" : "PUSH"
}, {
"hash" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13866",
"triggerID" : "0cd6ed4601354e8e30757ffc67335b5b1eb0410e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bd70e473f8a20f4d631980298ec201aa730713ae Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13865)
* 0cd6ed4601354e8e30757ffc67335b5b1eb0410e Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13866)
<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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1353867602
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9fb6bf2b503384db79feca453fcefa040f79c4b7 Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773)
<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
Re: [PR] [HUDI-5246] Added validation for Partition Path to not begin with "/" [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua closed pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
URL: https://github.com/apache/hudi/pull/7474
--
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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
xushiyan commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1364171946
i would suggest only do validation at config level not for the actual data (partition paths). This validation should not incur any performance hit.
--
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] hemanth-gowda-12 commented on pull request #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hemanth-gowda-12 commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1360359747
@nsivabalan thanks for looking, but looking at this again only from a Java client usage perspective, independent from Spark, Flink or Delta Streamer perspective, would the validation still work when using the Java Client if I followed the above guidelines?
--
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 #7474: [HUDI-5246] Added validation for Partition Path to not begin with "/"
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7474:
URL: https://github.com/apache/hudi/pull/7474#issuecomment-1358042104
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773",
"triggerID" : "9fb6bf2b503384db79feca453fcefa040f79c4b7",
"triggerType" : "PUSH"
}, {
"hash" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "bd70e473f8a20f4d631980298ec201aa730713ae",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9fb6bf2b503384db79feca453fcefa040f79c4b7 Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13773)
* bd70e473f8a20f4d631980298ec201aa730713ae 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