You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "boneanxs (via GitHub)" <gi...@apache.org> on 2023/10/20 03:19:55 UTC
[PR] [HUDI-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
boneanxs opened a new pull request, #9895:
URL: https://github.com/apache/hudi/pull/9895
### Change Logs
_Describe context and summary for this change. Highlight if any code was copied._
CreateIndex is added in [HUDI-4165](https://github.com/apache/hudi/pull/5761/files), and spark 3.3 also include this in [SPARK-36895](https://github.com/apache/spark/pull/34148). Since `CreateIndex` uses same package `org.apache.spark.sql.catalyst.plans.logical` in HUDI and Spark3.3, but params are not same. So it could introduce class conflict issues if we use it.
One simple modification to fix this issue is we directly move `CreateIndex` to a different package path, but I think it's not proper to do, given
1. We may not get benefit from Spark to perform other analyze rules for `CreateIndex`
2. Give us extra effort when moving to DSV2
3. Other 3rd packages rely on old path(like kyuubi hudi auth) requires additional changes.
4. HUDI always use `org.apache.spark.sql.catalyst.plans.logical` for logical plan(like `MergeInto`)
So here I still keep the same package path with Spark, but change
1. Use the same params like Spark, so there should be no class conflict
2. Since Spark2 doesn't have `org.apache.spark.sql.catalyst.analysis.FieldName` but `CreateIndex` requires, so we only support Index related commands from **Spark3.2**(I can also add support for Spark2 and Spark3.0, spark3.1, but not sure it's still needed or not)
3. Address TODO: resolve columns for CreateIndex during Analyze stage
### Impact
_Describe any public API or user-facing feature change or any performance impact._
Index related commands are not supported anymore below Spark3.2
### Risk level (write none, low medium or high below)
_If medium or high, explain what verification was done to mitigate the risks._
low
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change_
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### 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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782202507
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
}, {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b7afca087034b4e2b8f1b3030a1173a20e8c1899 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504)
* f462cf7b36016948969adb5083057a9ecbd6f794 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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1772317834
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782208359
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
}, {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b7afca087034b4e2b8f1b3030a1173a20e8c1899 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504)
* f462cf7b36016948969adb5083057a9ecbd6f794 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1772029929
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132 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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "boneanxs (via GitHub)" <gi...@apache.org>.
boneanxs commented on code in PR #9895:
URL: https://github.com/apache/hudi/pull/9895#discussion_r1372638346
##########
hudi-spark-datasource/hudi-spark/src/main/antlr4/org/apache/hudi/spark/sql/parser/HoodieSqlCommon.g4:
##########
@@ -135,51 +120,13 @@
nonReserved
: CALL
| COMPACTION
- | CREATE
- | DROP
- | EXISTS
- | FROM
- | IN
- | INDEX
- | INDEXES
- | IF
Review Comment:
No, I checked and only remove unneeded.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/parser/HoodieSqlCommonAstBuilder.scala:
##########
@@ -149,144 +149,4 @@ class HoodieSqlCommonAstBuilder(session: SparkSession, delegate: ParserInterface
private def typedVisit[T](ctx: ParseTree): T = {
ctx.accept(this).asInstanceOf[T]
}
-
- /**
Review Comment:
These codes are introduced from https://github.com/apache/hudi/pull/5761, maybe it's only for supporting Index commands
##########
hudi-spark-datasource/hudi-spark3.3.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_3ExtendedSqlAstBuilder.scala:
##########
@@ -3327,6 +3327,145 @@ class HoodieSpark3_3ExtendedSqlAstBuilder(conf: SQLConf, delegate: ParserInterfa
position = Option(ctx.colPosition).map(pos =>
UnresolvedFieldPosition(typedVisit[ColumnPosition](pos))))
}
+
+ /**
Review Comment:
Yea, it should be same
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -32,23 +31,21 @@ import org.apache.spark.sql.hudi.HoodieSqlCommonUtils.getTableLocation
import org.apache.spark.sql.{Row, SparkSession}
import java.util
-
import scala.collection.JavaConverters.{collectionAsScalaIterableConverter, mapAsJavaMapConverter}
case class CreateIndexCommand(table: CatalogTable,
indexName: String,
indexType: String,
ignoreIfExists: Boolean,
- columns: Seq[(Attribute, Map[String, String])],
- options: Map[String, String],
- override val output: Seq[Attribute]) extends IndexBaseCommand {
+ columns: Seq[(Seq[String], Map[String, String])],
+ options: Map[String, String]) extends IndexBaseCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
val columnsMap: java.util.LinkedHashMap[String, java.util.Map[String, String]] =
new util.LinkedHashMap[String, java.util.Map[String, String]]()
- columns.map(c => columnsMap.put(c._1.name, c._2.asJava))
+ columns.map(c => columnsMap.put(c._1.mkString("."), c._2.asJava))
Review Comment:
Now columns' name is `Seq[String]` instead of `Attribute`(`UnresolvedAttribute` to be more specific), since this pr will try to resolve column names
https://github.com/apache/hudi/blob/6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132/hudi-spark-datasource/hudi-spark3.2plus-common/src/main/scala/org/apache/spark/sql/hudi/analysis/HoodieSpark32PlusAnalysis.scala#L208
`UnresolvedAttribute` is no need anymore, so here I directly use `Seq[String]` to represent the column name here
##########
hudi-spark-datasource/hudi-spark3.3.x/src/main/antlr4/org/apache/hudi/spark/sql/parser/HoodieSqlBase.g4:
##########
@@ -29,5 +29,12 @@ statement
| createTableHeader ('(' colTypeList ')')? tableProvider?
createTableClauses
(AS? query)? #createTable
+ | CREATE INDEX (IF NOT EXISTS)? identifier ON TABLE?
Review Comment:
We cannot put the grammar in `HoodieSqlCommon.g4`, since we have to parse it in `HoodieSqlCommonAstBuilder` to build `CreateIndex`, while one param `FieldName` doesn't exist below Spark3.
We already have this for `time travel`, and grammar is rarely modified, so changing it like so is acceptable?
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestIndexSyntax.scala:
##########
@@ -28,59 +29,61 @@ import org.apache.spark.sql.hudi.command.{CreateIndexCommand, DropIndexCommand,
class TestIndexSyntax extends HoodieSparkSqlTestBase {
test("Test Create/Drop/Show/Refresh Index") {
- withTempDir { tmp =>
- Seq("cow", "mor").foreach { tableType =>
- val databaseName = "default"
- val tableName = generateTableName
- val basePath = s"${tmp.getCanonicalPath}/$tableName"
- spark.sql(
- s"""
- |create table $tableName (
- | id int,
- | name string,
- | price double,
- | ts long
- |) using hudi
- | options (
- | primaryKey ='id',
- | type = '$tableType',
- | preCombineField = 'ts'
- | )
- | partitioned by(ts)
- | location '$basePath'
+ if (HoodieSparkUtils.gteqSpark3_2) {
Review Comment:
yea, will fix it as well
##########
hudi-spark-datasource/hudi-spark3.2.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_2ExtendedSqlAstBuilder.scala:
##########
@@ -3317,6 +3317,145 @@ class HoodieSpark3_2ExtendedSqlAstBuilder(conf: SQLConf, delegate: ParserInterfa
position = Option(ctx.colPosition).map(pos =>
UnresolvedFieldPosition(typedVisit[ColumnPosition](pos))))
}
+
+ /**
Review Comment:
Yes
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1780594113
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414)
* b7afca087034b4e2b8f1b3030a1173a20e8c1899 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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1783709069
Hi @boneanxs , I'm working on Spark 3.5 support and noticed 2 new failures after cherry-picking your commit. I tried porting this changes for Spark 3.5 but it didn't help. I'm still looking into the test failures but would appreciate if you can help take a look as well! My PR: #9717
- Failure 1
```
2023-10-28T02:40:26.0783904Z - Test Create/Show/Drop Secondary Index *** FAILED ***
2023-10-28T02:40:26.0785667Z org.apache.spark.sql.AnalysisException: CreateIndex is not supported in this table default.h0.
2023-10-28T02:40:26.0787735Z at org.apache.spark.sql.errors.QueryCompilationErrors$.tableIndexNotSupportedError(QueryCompilationErrors.scala:3259)
```
- Failure 2
```
T03:03:39.6074356Z - Test Create/Drop/Show/Refresh Index *** FAILED ***
2023-10-28T03:03:39.6077467Z java.lang.ClassCastException: org.apache.spark.sql.catalyst.plans.logical.CreateIndex cannot be cast to org.apache.spark.sql.hudi.command.CreateIndexCommand
2023-10-28T03:03:39.6079990Z at org.apache.spark.sql.hudi.command.index.TestIndexSyntax.$anonfun$new$3(TestIndexSyntax.scala:67)
2023-10-28T03:03:39.6081642Z at scala.collection.immutable.List.foreach(List.scala:431)
2023-10-28T03:03:39.6143392Z at org.apache.spark.sql.hudi.command.index.TestIndexSyntax.$anonfun$new$2(TestIndexSyntax.scala:34)
```
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782470903
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f462cf7b36016948969adb5083057a9ecbd6f794 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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua merged PR #9895:
URL: https://github.com/apache/hudi/pull/9895
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782294429
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
}, {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f462cf7b36016948969adb5083057a9ecbd6f794 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #9895:
URL: https://github.com/apache/hudi/pull/9895#discussion_r1369644417
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestIndexSyntax.scala:
##########
@@ -28,59 +29,61 @@ import org.apache.spark.sql.hudi.command.{CreateIndexCommand, DropIndexCommand,
class TestIndexSyntax extends HoodieSparkSqlTestBase {
test("Test Create/Drop/Show/Refresh Index") {
- withTempDir { tmp =>
- Seq("cow", "mor").foreach { tableType =>
- val databaseName = "default"
- val tableName = generateTableName
- val basePath = s"${tmp.getCanonicalPath}/$tableName"
- spark.sql(
- s"""
- |create table $tableName (
- | id int,
- | name string,
- | price double,
- | ts long
- |) using hudi
- | options (
- | primaryKey ='id',
- | type = '$tableType',
- | preCombineField = 'ts'
- | )
- | partitioned by(ts)
- | location '$basePath'
+ if (HoodieSparkUtils.gteqSpark3_2) {
Review Comment:
Looks like `TestSecondaryIndex` should also have a precondition on the spark version.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -32,23 +31,21 @@ import org.apache.spark.sql.hudi.HoodieSqlCommonUtils.getTableLocation
import org.apache.spark.sql.{Row, SparkSession}
import java.util
-
import scala.collection.JavaConverters.{collectionAsScalaIterableConverter, mapAsJavaMapConverter}
case class CreateIndexCommand(table: CatalogTable,
indexName: String,
indexType: String,
ignoreIfExists: Boolean,
- columns: Seq[(Attribute, Map[String, String])],
- options: Map[String, String],
- override val output: Seq[Attribute]) extends IndexBaseCommand {
+ columns: Seq[(Seq[String], Map[String, String])],
+ options: Map[String, String]) extends IndexBaseCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
val columnsMap: java.util.LinkedHashMap[String, java.util.Map[String, String]] =
new util.LinkedHashMap[String, java.util.Map[String, String]]()
- columns.map(c => columnsMap.put(c._1.name, c._2.asJava))
+ columns.map(c => columnsMap.put(c._1.mkString("."), c._2.asJava))
Review Comment:
Why change this? for nested fields?
##########
hudi-spark-datasource/hudi-spark3.3.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_3ExtendedSqlAstBuilder.scala:
##########
@@ -3327,6 +3327,145 @@ class HoodieSpark3_3ExtendedSqlAstBuilder(conf: SQLConf, delegate: ParserInterfa
position = Option(ctx.colPosition).map(pos =>
UnresolvedFieldPosition(typedVisit[ColumnPosition](pos))))
}
+
+ /**
Review Comment:
I assume the SQL parsing of INDEX SQL statement should not be different across Spark versions.
##########
hudi-spark-datasource/hudi-spark3.2.x/src/main/scala/org/apache/spark/sql/parser/HoodieSpark3_2ExtendedSqlAstBuilder.scala:
##########
@@ -3317,6 +3317,145 @@ class HoodieSpark3_2ExtendedSqlAstBuilder(conf: SQLConf, delegate: ParserInterfa
position = Option(ctx.colPosition).map(pos =>
UnresolvedFieldPosition(typedVisit[ColumnPosition](pos))))
}
+
+ /**
Review Comment:
Got it. So at least CreateIndex is still supported in Spark 3.2.
##########
hudi-spark-datasource/hudi-spark/src/main/antlr4/org/apache/hudi/spark/sql/parser/HoodieSqlCommon.g4:
##########
@@ -135,51 +120,13 @@
nonReserved
: CALL
| COMPACTION
- | CREATE
- | DROP
- | EXISTS
- | FROM
- | IN
- | INDEX
- | INDEXES
- | IF
Review Comment:
Do we still need some of these tokens for other SQL statements?
##########
hudi-spark-datasource/hudi-spark3.3.x/src/main/antlr4/org/apache/hudi/spark/sql/parser/HoodieSqlBase.g4:
##########
@@ -29,5 +29,12 @@ statement
| createTableHeader ('(' colTypeList ')')? tableProvider?
createTableClauses
(AS? query)? #createTable
+ | CREATE INDEX (IF NOT EXISTS)? identifier ON TABLE?
Review Comment:
Could we still maintain the grammar in a single place for all Spark versions, but fail the logical plan of INDEX SQL statement in Spark 3.1 and below, so the grammar can be easily maintained?
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/parser/HoodieSqlCommonAstBuilder.scala:
##########
@@ -149,144 +149,4 @@ class HoodieSqlCommonAstBuilder(session: SparkSession, delegate: ParserInterface
private def typedVisit[T](ctx: ParseTree): T = {
ctx.accept(this).asInstanceOf[T]
}
-
- /**
Review Comment:
Are all of this code copied from Spark 3.3+? Wondering if the original PR introducing this intends to support CreateIndex for Spark 3.2 and below. cc @huberylee
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #9895:
URL: https://github.com/apache/hudi/pull/9895#discussion_r1372688660
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -32,23 +31,21 @@ import org.apache.spark.sql.hudi.HoodieSqlCommonUtils.getTableLocation
import org.apache.spark.sql.{Row, SparkSession}
import java.util
-
import scala.collection.JavaConverters.{collectionAsScalaIterableConverter, mapAsJavaMapConverter}
case class CreateIndexCommand(table: CatalogTable,
indexName: String,
indexType: String,
ignoreIfExists: Boolean,
- columns: Seq[(Attribute, Map[String, String])],
- options: Map[String, String],
- override val output: Seq[Attribute]) extends IndexBaseCommand {
+ columns: Seq[(Seq[String], Map[String, String])],
+ options: Map[String, String]) extends IndexBaseCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
val columnsMap: java.util.LinkedHashMap[String, java.util.Map[String, String]] =
new util.LinkedHashMap[String, java.util.Map[String, String]]()
- columns.map(c => columnsMap.put(c._1.name, c._2.asJava))
+ columns.map(c => columnsMap.put(c._1.mkString("."), c._2.asJava))
Review Comment:
Got 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
Re: [PR] [HUDI-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1776548151
cc @codope
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782691713
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f462cf7b36016948969adb5083057a9ecbd6f794 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #9895:
URL: https://github.com/apache/hudi/pull/9895#discussion_r1372696612
##########
hudi-spark-datasource/hudi-spark3.3.x/src/main/antlr4/org/apache/hudi/spark/sql/parser/HoodieSqlBase.g4:
##########
@@ -29,5 +29,12 @@ statement
| createTableHeader ('(' colTypeList ')')? tableProvider?
createTableClauses
(AS? query)? #createTable
+ | CREATE INDEX (IF NOT EXISTS)? identifier ON TABLE?
Review Comment:
Got it. @codope is working on functional indexes in Hudi and may extend the grammar here. So I'm thinking if it's not too hard to put the grammar in a common place, we should do that.
--
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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1780950068
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b7afca087034b4e2b8f1b3030a1173a20e8c1899 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1782531655
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525",
"triggerID" : "f462cf7b36016948969adb5083057a9ecbd6f794",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f462cf7b36016948969adb5083057a9ecbd6f794 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20525)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1780663206
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
}, {
"hash" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504",
"triggerID" : "b7afca087034b4e2b8f1b3030a1173a20e8c1899",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414)
* b7afca087034b4e2b8f1b3030a1173a20e8c1899 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20504)
<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-6963] Fix class conflict of CreateIndex from Spark3.3 [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #9895:
URL: https://github.com/apache/hudi/pull/9895#issuecomment-1772035508
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414",
"triggerID" : "6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6eb38c5b78e4f53a7d21c15f5bb63cd528bc5132 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20414)
<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