You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "waywtdcc (via GitHub)" <gi...@apache.org> on 2023/05/16 05:34:53 UTC
[GitHub] [hudi] waywtdcc opened a new pull request, #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
waywtdcc opened a new pull request, #8721:
URL: https://github.com/apache/hudi/pull/8721
### Change Logs
Fix Spark reads the deleted data whose _hoodie_operation is D
### Impact
### Contributor's checklist
- [X ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [X] 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] danny0405 commented on a diff in pull request #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8721:
URL: https://github.com/apache/hudi/pull/8721#discussion_r1196171746
##########
hudi-common/src/main/java/org/apache/hudi/common/model/BaseAvroPayload.java:
##########
@@ -85,13 +85,20 @@ public boolean canProduceSentinel() {
*/
protected static boolean isDeleteRecord(GenericRecord genericRecord) {
final String isDeleteKey = HoodieRecord.HOODIE_IS_DELETED_FIELD;
+ final String isDeleteKeyMeta = HoodieRecord.OPERATION_METADATA_FIELD;
// Modify to be compatible with new version Avro.
Review Comment:
`isDeleteKeyMeta` -> `operationKey`
--
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] waywtdcc commented on pull request #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "waywtdcc (via GitHub)" <gi...@apache.org>.
waywtdcc commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1549061495
@hudi-bot run azure
--
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 #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8721:
URL: https://github.com/apache/hudi/pull/8721#discussion_r1197636500
##########
hudi-common/src/main/java/org/apache/hudi/common/model/BaseAvroPayload.java:
##########
@@ -85,13 +85,20 @@ public boolean canProduceSentinel() {
*/
protected static boolean isDeleteRecord(GenericRecord genericRecord) {
final String isDeleteKey = HoodieRecord.HOODIE_IS_DELETED_FIELD;
+ final String isDeleteKeyMeta = HoodieRecord.OPERATION_METADATA_FIELD;
// Modify to be compatible with new version Avro.
// The new version Avro throws for GenericRecord.get if the field name
// does not exist in the schema.
- if (genericRecord.getSchema().getField(isDeleteKey) == null) {
- return false;
+ if (genericRecord.getSchema().getField(isDeleteKey) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKey);
+ return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
}
- Object deleteMarker = genericRecord.get(isDeleteKey);
- return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
+
+ if (genericRecord.getSchema().getField(isDeleteKeyMeta) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKeyMeta);
+ return (deleteMarker != null && "D".equals(deleteMarker.toString()));
Review Comment:
It affects the usage because the `getInsertValue` returns nothing after your change, we still need the whole payload for retraction.
--
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 #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8721:
URL: https://github.com/apache/hudi/pull/8721#discussion_r1194710953
##########
hudi-common/src/main/java/org/apache/hudi/common/model/BaseAvroPayload.java:
##########
@@ -85,13 +85,20 @@ public boolean canProduceSentinel() {
*/
protected static boolean isDeleteRecord(GenericRecord genericRecord) {
final String isDeleteKey = HoodieRecord.HOODIE_IS_DELETED_FIELD;
+ final String isDeleteKeyMeta = HoodieRecord.OPERATION_METADATA_FIELD;
// Modify to be compatible with new version Avro.
// The new version Avro throws for GenericRecord.get if the field name
// does not exist in the schema.
- if (genericRecord.getSchema().getField(isDeleteKey) == null) {
- return false;
+ if (genericRecord.getSchema().getField(isDeleteKey) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKey);
+ return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
}
- Object deleteMarker = genericRecord.get(isDeleteKey);
- return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
+
+ if (genericRecord.getSchema().getField(isDeleteKeyMeta) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKeyMeta);
+ return (deleteMarker != null && "D".equals(deleteMarker.toString()));
Review Comment:
Nice catch, can you write a test case for Spark then?
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1837025637
Close because it is fixed in: https://github.com/apache/hudi/pull/10219
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "waywtdcc (via GitHub)" <gi...@apache.org>.
waywtdcc commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1751884980
> Should be feasible if the Merger API also supports delete, but we should not always mask out the deletes because some readers like the FLink streaming reader atually needs to emit these deletion messages, so I guess put the handling into the FileGroup reader is more reasonable.
If the delete message wants to be sent, why does BaseAvroPayload.java forcefully delete the data with HOODIE_IS_DELETED_FIELD set to true now? Feeling inconsistent
--
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] yihua commented on pull request #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1732672773
What about the support of handing '_hoodie_operation' meta field with new merge API and Spark record merger?
--
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 #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1549073910
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"triggerType" : "PUSH"
}, {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1549061495",
"triggerType" : "MANUAL"
} ]
}-->
## CI report:
* 5842bf5508e0b3ee69bac447930b2f79e8dd73b2 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] danny0405 commented on a diff in pull request #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8721:
URL: https://github.com/apache/hudi/pull/8721#discussion_r1196184653
##########
hudi-common/src/main/java/org/apache/hudi/common/model/BaseAvroPayload.java:
##########
@@ -85,13 +85,20 @@ public boolean canProduceSentinel() {
*/
protected static boolean isDeleteRecord(GenericRecord genericRecord) {
final String isDeleteKey = HoodieRecord.HOODIE_IS_DELETED_FIELD;
+ final String isDeleteKeyMeta = HoodieRecord.OPERATION_METADATA_FIELD;
// Modify to be compatible with new version Avro.
// The new version Avro throws for GenericRecord.get if the field name
// does not exist in the schema.
- if (genericRecord.getSchema().getField(isDeleteKey) == null) {
- return false;
+ if (genericRecord.getSchema().getField(isDeleteKey) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKey);
+ return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
}
- Object deleteMarker = genericRecord.get(isDeleteKey);
- return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
+
+ if (genericRecord.getSchema().getField(isDeleteKeyMeta) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKeyMeta);
+ return (deleteMarker != null && "D".equals(deleteMarker.toString()));
Review Comment:
I'm afraid we can not ignore the whole payload bytes while the `_hoodie_operation` is `D`, such as in Flink: https://github.com/apache/hudi/blob/014168b948a5a1ca88ea3e8cca213dfd46cef3f4/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/format/mor/MergeOnReadInputFormat.java#L820, there are use cases when the record is a DELETE, it is still needs to be emiit representing a retraction msg.
So, we suggest to fix the `_hoodie_operation` meta field handing just in the Spark snapshot relation, just like what we do specifically in the inputformat of Flink source.
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1752224586
> > Should be feasible if the Merger API also supports delete, but we should not always mask out the deletes because some readers like the FLink streaming reader atually needs to emit these deletion messages, so I guess put the handling into the FileGroup reader is more reasonable.
>
> If the delete message wants to be sent, why does BaseAvroPayload.java forcefully delete the data with HOODIE_IS_DELETED_FIELD set to true now? Feeling inconsistent
_hoodie_operation with D is used for stream messages retraction while HOODIE_IS_DELETED_FIELD is used for hard delete in data ingestion.
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1800869553
@beyond1920 You can just take 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] hudi-bot commented on pull request #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1549255646
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"triggerType" : "PUSH"
}, {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1549061495",
"triggerType" : "MANUAL"
}, {
"hash" : "48da8aa5151c36a139af6546594242847fcea6bc",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "48da8aa5151c36a139af6546594242847fcea6bc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5842bf5508e0b3ee69bac447930b2f79e8dd73b2 UNKNOWN
* 48da8aa5151c36a139af6546594242847fcea6bc 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] waywtdcc commented on a diff in pull request #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
Posted by "waywtdcc (via GitHub)" <gi...@apache.org>.
waywtdcc commented on code in PR #8721:
URL: https://github.com/apache/hudi/pull/8721#discussion_r1197307501
##########
hudi-common/src/main/java/org/apache/hudi/common/model/BaseAvroPayload.java:
##########
@@ -85,13 +85,20 @@ public boolean canProduceSentinel() {
*/
protected static boolean isDeleteRecord(GenericRecord genericRecord) {
final String isDeleteKey = HoodieRecord.HOODIE_IS_DELETED_FIELD;
+ final String isDeleteKeyMeta = HoodieRecord.OPERATION_METADATA_FIELD;
// Modify to be compatible with new version Avro.
// The new version Avro throws for GenericRecord.get if the field name
// does not exist in the schema.
- if (genericRecord.getSchema().getField(isDeleteKey) == null) {
- return false;
+ if (genericRecord.getSchema().getField(isDeleteKey) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKey);
+ return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
}
- Object deleteMarker = genericRecord.get(isDeleteKey);
- return (deleteMarker instanceof Boolean && (boolean) deleteMarker);
+
+ if (genericRecord.getSchema().getField(isDeleteKeyMeta) != null) {
+ Object deleteMarker = genericRecord.get(isDeleteKeyMeta);
+ return (deleteMarker != null && "D".equals(deleteMarker.toString()));
Review Comment:
> I'm afraid we can not ignore the whole payload bytes while the `_hoodie_operation` is `D`, such as in Flink:
>
> https://github.com/apache/hudi/blob/014168b948a5a1ca88ea3e8cca213dfd46cef3f4/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/format/mor/MergeOnReadInputFormat.java#L820
>
> , there are use cases when the record is a DELETE, it is still needs to be emiit representing a retraction msg.
> So, we suggest to fix the `_hoodie_operation` meta field handing just in the Spark snapshot relation, just like what we do specifically in the inputformat of Flink source.
I don't think this affects the use of this method. And it is better to put this logic in the getInsertValue method of BaseAvroPayload
--
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 pull request #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1736629662
Should be feasible if the Merger API also supports delete, but we should not always mask out the deletes because some readers like the FLink streaming reader atually needs to emit these deletion messages, so I guess put the handling into the FileGroup reader is more reasonable.
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 closed pull request #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
URL: https://github.com/apache/hudi/pull/8721
--
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-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source [hudi]
Posted by "beyond1920 (via GitHub)" <gi...@apache.org>.
beyond1920 commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1798442939
@waywtdcc Hello, thanks for contribution. Are you still working on this pr?
If not, do you mind I take 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] hudi-bot commented on pull request #8721: [HUDI-6217]Fix Spark reads the deleted data whose _hoodie_operation is D
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1549025868
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5842bf5508e0b3ee69bac447930b2f79e8dd73b2 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 #8721: [HUDI-6217] Support handing '_hoodie_operation' meta field for Spark snapshot source
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8721:
URL: https://github.com/apache/hudi/pull/8721#issuecomment-1552367945
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"triggerType" : "PUSH"
}, {
"hash" : "5842bf5508e0b3ee69bac447930b2f79e8dd73b2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1549061495",
"triggerType" : "MANUAL"
}, {
"hash" : "48da8aa5151c36a139af6546594242847fcea6bc",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "48da8aa5151c36a139af6546594242847fcea6bc",
"triggerType" : "PUSH"
}, {
"hash" : "04cba645ae48a797887017247347d799a5e6e423",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "04cba645ae48a797887017247347d799a5e6e423",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5842bf5508e0b3ee69bac447930b2f79e8dd73b2 UNKNOWN
* 48da8aa5151c36a139af6546594242847fcea6bc UNKNOWN
* 04cba645ae48a797887017247347d799a5e6e423 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