You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "jonvex (via GitHub)" <gi...@apache.org> on 2024/03/06 01:49:42 UTC
[PR] [HUDI-6472] fix spark sql does not ignore case [hudi]
jonvex opened a new pull request, #10826:
URL: https://github.com/apache/hudi/pull/10826
### Change Logs
https://github.com/apache/hudi/pull/10582
with the following changes:
- HoodieSpark32PlusAnalysis: made this change much less complex
- correct capitalization of partition column names for keygen
### Impact
allow merge into to ignore case
### 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
Re: [PR] [HUDI-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1981594399
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
}, {
"hash" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22818",
"triggerID" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 88b603bae99b82b28a2a2f3b4fa5372bb570eb5c Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22818)
<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-6472] fix spark sql does not ignore case [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513874685
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/TestInsertTable.scala:
##########
@@ -2448,6 +2449,50 @@ class TestInsertTable extends HoodieSparkSqlTestBase {
})
}
+ test("Test query with Foldable Propagation expression") {
+ withRecordType(Seq(HoodieRecordType.AVRO))(withTempDir { tmp =>
Review Comment:
Remove `withRecordType(Seq(HoodieRecordType.AVRO))(` as it's not required.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "beyond1920 (via GitHub)" <gi...@apache.org>.
beyond1920 commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1980284804
@jonvex It seems a little heavy to use the optimizer here just for case insensitive.
Besides, if wrap an optimize phase here, user might missed information about plan conversion in spark sql WEB UI, right?
Is there any better solution?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1514891780
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/TestInsertTable.scala:
##########
@@ -2448,6 +2449,50 @@ class TestInsertTable extends HoodieSparkSqlTestBase {
})
}
+ test("Test query with Foldable Propagation expression") {
Review Comment:
Not sure what to change this too. It deals with a specific spark optimization
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1982004268
> @yihua @danny0405 @beyond1920:
>
> Without adding the optimizer, we get the exception: `java.lang.RuntimeException: After applying rule org.apache.spark.sql.catalyst.optimizer.FoldablePropagation in batch Operator Optimization before Inferring Filters, the structural integrity of the plan is broken.`
>
> Here is the plan before and after the FoldablePropagation optimizer step when running the test "Test ignoring case for MOR table":
>
> ```
> Before:
> Project [ID#10, NAME#11, price#12, TS#13, dt#14]
> +- Join LeftOuter, (id#20 = id#10)
> :- Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
> : +- OneRowRelation
> +- Project [ID#20]
> +- Relation default.h0[_hoodie_commit_time#15,_hoodie_commit_seqno#16,_hoodie_record_key#17,_hoodie_partition_path#18,_hoodie_file_name#19,ID#20,NAME#21,price#22,TS#23L,dt#24] org.apache.hudi.EmptyRelation@67cd84f9
>
> After:
> Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
> +- Join LeftOuter, (id#20 = 1)
> :- Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
> : +- OneRowRelation
> +- Project [ID#20]
> +- Relation default.h0[_hoodie_commit_time#15,_hoodie_commit_seqno#16,_hoodie_record_key#17,_hoodie_partition_path#18,_hoodie_file_name#19,ID#20,NAME#21,price#22,TS#23L,dt#24] org.apache.hudi.EmptyRelation@67cd84f9
> ```
>
> @KnightChess can we avoid running the optimizer here if we find a way to rename all id#10 to ID#10 and not just the outermost projection?
If the optimizer overhead is not low and makes the plan obscure, do we really need this fix? We can explicitly mention to the user in the docs that the SQL query has to be field names that have to be case-sensitive. Or is this a really critical issue?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1980054774
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807)
<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-6472] fix spark sql does not ignore case [hudi]
Posted by "beyond1920 (via GitHub)" <gi...@apache.org>.
beyond1920 commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513982842
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/InsertIntoHoodieTableCommand.scala:
##########
@@ -95,7 +95,9 @@ object InsertIntoHoodieTableCommand extends Logging with ProvidesHoodieConfig wi
}
val config = buildHoodieInsertConfig(catalogTable, sparkSession, isOverWritePartition, isOverWriteTable, partitionSpec, extraOptions, staticOverwritePartitionPathOpt)
- val alignedQuery = alignQueryOutput(query, catalogTable, partitionSpec, sparkSession.sessionState.conf)
+ val optimizer = sparkSession.sessionState.optimizer
+ val optimizerPlan = optimizer.execute(query)
+ val alignedQuery = alignQueryOutput(optimizerPlan, catalogTable, partitionSpec, sparkSession.sessionState.conf)
Review Comment:
It's seems a little heavy to use optimizer just for case insensitive.
Besides, if wrap an optimize phase here, user might missed something in spark sql WEB UI.
Is there any better solution?
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/InsertIntoHoodieTableCommand.scala:
##########
@@ -95,7 +95,9 @@ object InsertIntoHoodieTableCommand extends Logging with ProvidesHoodieConfig wi
}
val config = buildHoodieInsertConfig(catalogTable, sparkSession, isOverWritePartition, isOverWriteTable, partitionSpec, extraOptions, staticOverwritePartitionPathOpt)
- val alignedQuery = alignQueryOutput(query, catalogTable, partitionSpec, sparkSession.sessionState.conf)
+ val optimizer = sparkSession.sessionState.optimizer
+ val optimizerPlan = optimizer.execute(query)
+ val alignedQuery = alignQueryOutput(optimizerPlan, catalogTable, partitionSpec, sparkSession.sessionState.conf)
Review Comment:
It seems a little heavy to use optimizer just for case insensitive.
Besides, if wrap an optimize phase here, user might missed something in spark sql WEB UI.
Is there any better solution?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1983930177
@danny0405 the root cause of the case insensitive problem is that in `HoodieSpark32PlusAnalysis` I imported `org.apache.spark.sql.catalyst.analysis.SimpleAnalyzer.resolveExpressionByPlanChildren`. SimpleAnalyzer is a test class that is hardcoded as case sensitive. After using the resolveExpressionByPlanChildren method from our resolver, we face this issue with the optimizer validation failing. My guess is that we are not renaming the column names correctly. It might be possible to find a solution without renaming the columns but I would guess that it will take a lot more effort than just renaming the columns throughout the entire plan.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1981362523
@yihua @danny0405 @beyond1920:
Without adding the optimizer, we get the exception:
`java.lang.RuntimeException: After applying rule org.apache.spark.sql.catalyst.optimizer.FoldablePropagation in batch Operator Optimization before Inferring Filters, the structural integrity of the plan is broken.`
Here is the plan before and after the FoldablePropagation step when running the test "Test ignoring case for MOR table":
```
Before:
Project [ID#10, NAME#11, price#12, TS#13, dt#14]
+- Join LeftOuter, (id#20 = id#10)
:- Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
: +- OneRowRelation
+- Project [ID#20]
+- Relation default.h0[_hoodie_commit_time#15,_hoodie_commit_seqno#16,_hoodie_record_key#17,_hoodie_partition_path#18,_hoodie_file_name#19,ID#20,NAME#21,price#22,TS#23L,dt#24] org.apache.hudi.EmptyRelation@67cd84f9
After:
Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
+- Join LeftOuter, (id#20 = 1)
:- Project [1 AS id#10, a1 AS NAME#11, 111 AS price#12, 1111 AS ts#13, 2021-05-05 AS DT#14]
: +- OneRowRelation
+- Project [ID#20]
+- Relation default.h0[_hoodie_commit_time#15,_hoodie_commit_seqno#16,_hoodie_record_key#17,_hoodie_partition_path#18,_hoodie_file_name#19,ID#20,NAME#21,price#22,TS#23L,dt#24] org.apache.hudi.EmptyRelation@67cd84f9
```
@KnightChess can we avoid running the optimizer here if we find a way to rename all id#10 and not just the outermost projection?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1984864777
> SimpleAnalyzer is a test class that is hardcoded as case sensitive.
I kind of think we should not introduce a test code snippet that is inconsistent with Spark's regular norms. Is it possible we extend the `Analyzer` class and implement our own analyzer with case sensitiveness as false?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513876839
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/InsertIntoHoodieTableCommand.scala:
##########
@@ -95,7 +95,9 @@ object InsertIntoHoodieTableCommand extends Logging with ProvidesHoodieConfig wi
}
val config = buildHoodieInsertConfig(catalogTable, sparkSession, isOverWritePartition, isOverWriteTable, partitionSpec, extraOptions, staticOverwritePartitionPathOpt)
- val alignedQuery = alignQueryOutput(query, catalogTable, partitionSpec, sparkSession.sessionState.conf)
+ val optimizer = sparkSession.sessionState.optimizer
+ val optimizerPlan = optimizer.execute(query)
+ val alignedQuery = alignQueryOutput(optimizerPlan, catalogTable, partitionSpec, sparkSession.sessionState.conf)
Review Comment:
is this required for case insensitivity or this is for performance optimization?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1514821447
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/InsertIntoHoodieTableCommand.scala:
##########
@@ -95,7 +95,9 @@ object InsertIntoHoodieTableCommand extends Logging with ProvidesHoodieConfig wi
}
val config = buildHoodieInsertConfig(catalogTable, sparkSession, isOverWritePartition, isOverWriteTable, partitionSpec, extraOptions, staticOverwritePartitionPathOpt)
- val alignedQuery = alignQueryOutput(query, catalogTable, partitionSpec, sparkSession.sessionState.conf)
+ val optimizer = sparkSession.sessionState.optimizer
+ val optimizerPlan = optimizer.execute(query)
+ val alignedQuery = alignQueryOutput(optimizerPlan, catalogTable, partitionSpec, sparkSession.sessionState.conf)
Review Comment:
This is required. I don't know how @KnightChess figured this out; I'm impressed. You can see more detail in https://github.com/apache/hudi/pull/10582, but basically the optimizer checks to make sure the output of the plan doesn't change after each optimization step. Because of the name changes, the FoldablePropagation step will fail due to pulling up some expressions into the projection where we do renaming. We now run the optimizer on the query before doing the renaming. The expressions have already been pulled up, so when that optimization step runs again later, it won't do anything.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513878332
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/MergeIntoHoodieTableCommand.scala:
##########
@@ -372,17 +374,23 @@ case class MergeIntoHoodieTableCommand(mergeInto: MergeIntoTable) extends Hoodie
// In case when we're not adding new columns we need to make sure that the casing of the key attributes'
// matches to that one of the target table. This is necessary b/c unlike Spark, Avro is case-sensitive
// and therefore would fail downstream if case of corresponding columns don't match
+ val partitionColumns = hoodieCatalogTable.tableConfig.getPartitionFieldProp.split(",").toSeq
val existingAttributes = existingAttributesMap.map(_._1)
- val adjustedSourceTableOutput = projectedJoinOutput.map { attr =>
+ val adjustedSourceTableOutput = projectedJoinOptimizerOutput.map { attr =>
existingAttributes.find(keyAttr => resolver(keyAttr.name, attr.name)) match {
// To align the casing we just rename the attribute to match that one of the
// target table
case Some(keyAttr) => attr.withName(keyAttr.name)
- case _ => attr
+ // additional check for partition columns because they are not required,
+ // but we still care about casing because of keygenerator
+ case _ => partitionColumns.find(colName => resolver(colName, attr.name)) match {
Review Comment:
can we add a test around this case?
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/TestInsertTable.scala:
##########
@@ -2448,6 +2449,50 @@ class TestInsertTable extends HoodieSparkSqlTestBase {
})
}
+ test("Test query with Foldable Propagation expression") {
Review Comment:
Make the test name more readable
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-3114145579
@KnightChess @jonvex is this not an issue any more with newer version of Spark?
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1981390865
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
}, {
"hash" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807)
* 88b603bae99b82b28a2a2f3b4fa5372bb570eb5c 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
Re: [PR] [HUDI-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1981403693
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
}, {
"hash" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22818",
"triggerID" : "88b603bae99b82b28a2a2f3b4fa5372bb570eb5c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807)
* 88b603bae99b82b28a2a2f3b4fa5372bb570eb5c Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22818)
<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-6472] fix spark sql does not ignore case [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1982312119
We could if we rewrite the logical plan manually, just write a shuttle class to iterator over all the nodes from bottem to the outmost one, and checks the name case sensitiveness for all the fields.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513958254
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/InsertIntoHoodieTableCommand.scala:
##########
@@ -95,7 +95,9 @@ object InsertIntoHoodieTableCommand extends Logging with ProvidesHoodieConfig wi
}
val config = buildHoodieInsertConfig(catalogTable, sparkSession, isOverWritePartition, isOverWriteTable, partitionSpec, extraOptions, staticOverwritePartitionPathOpt)
- val alignedQuery = alignQueryOutput(query, catalogTable, partitionSpec, sparkSession.sessionState.conf)
+ val optimizer = sparkSession.sessionState.optimizer
+ val optimizerPlan = optimizer.execute(query)
+ val alignedQuery = alignQueryOutput(optimizerPlan, catalogTable, partitionSpec, sparkSession.sessionState.conf)
Review Comment:
Yeah, we need some clarification here.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1979962655
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22807)
<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-6472] fix spark sql does not ignore case [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-1979957408
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c2acd76a0d937ac926d5fdabafbfc1d66b61e2f 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
Re: [PR] [HUDI-6472] fix spark sql does not ignore case [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1513954128
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/MergeIntoHoodieTableCommand.scala:
##########
@@ -346,12 +346,14 @@ case class MergeIntoHoodieTableCommand(mergeInto: MergeIntoTable) extends Hoodie
Project(incomingDataCols, joinData)
}
- val projectedJoinOutput = projectedJoinPlan.output
+ val optimizer = sparkSession.sessionState.optimizer
+ val projectedJoinOptimizerPlan = optimizer.execute(projectedJoinPlan)
+ val projectedJoinOptimizerOutput = projectedJoinOptimizerPlan.output
Review Comment:
So the `optimizer.execute` is the critical step to fix the case sensitiveness.
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on code in PR #10826:
URL: https://github.com/apache/hudi/pull/10826#discussion_r1514833811
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/MergeIntoHoodieTableCommand.scala:
##########
@@ -372,17 +374,23 @@ case class MergeIntoHoodieTableCommand(mergeInto: MergeIntoTable) extends Hoodie
// In case when we're not adding new columns we need to make sure that the casing of the key attributes'
// matches to that one of the target table. This is necessary b/c unlike Spark, Avro is case-sensitive
// and therefore would fail downstream if case of corresponding columns don't match
+ val partitionColumns = hoodieCatalogTable.tableConfig.getPartitionFieldProp.split(",").toSeq
val existingAttributes = existingAttributesMap.map(_._1)
- val adjustedSourceTableOutput = projectedJoinOutput.map { attr =>
+ val adjustedSourceTableOutput = projectedJoinOptimizerOutput.map { attr =>
existingAttributes.find(keyAttr => resolver(keyAttr.name, attr.name)) match {
// To align the casing we just rename the attribute to match that one of the
// target table
case Some(keyAttr) => attr.withName(keyAttr.name)
- case _ => attr
+ // additional check for partition columns because they are not required,
+ // but we still care about casing because of keygenerator
+ case _ => partitionColumns.find(colName => resolver(colName, attr.name)) match {
Review Comment:
The test "Test ignoring case for MOR table" hits this case. Partition column is not required for the join so we didn't fix the naming like we did for precombine and recordkey. However, capitalization still matters if it is included because we do keygeneration in avro. So I added this check to adjust the partition col names if they are part of the assignment
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "KnightChess (via GitHub)" <gi...@apache.org>.
KnightChess commented on PR #10826:
URL: https://github.com/apache/hudi/pull/10826#issuecomment-2130901067
I have create a mr to solve this question in spark to deal with `FoldablePropagation issue`, I think FoldablePropagation should use origin schema.
https://github.com/apache/spark/pull/46742
--
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-6472] fix spark sql does not ignore case [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex closed pull request #10826: [HUDI-6472] fix spark sql does not ignore case
URL: https://github.com/apache/hudi/pull/10826
--
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