-
Notifications
You must be signed in to change notification settings - Fork 490
[spark] Add sparksql alter table partial support for set/remove table properties #2437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
flink ut failed with |
fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/SparkCatalog.scala
Outdated
Show resolved
Hide resolved
fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/SparkCatalog.scala
Outdated
Show resolved
Hide resolved
| checkAnswer(sql("SHOW DATABASES"), Row(DEFAULT_DATABASE) :: Nil) | ||
| } | ||
|
|
||
| test("Catalog: set/remove table properties") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it'll be nice to have a separate SparkTableChangeTest, which includes all Alter Table tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
above comments suggests add all DDL tests into one suite.
fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/SparkCatalogTest.scala
Outdated
Show resolved
Hide resolved
fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/SparkCatalogTest.scala
Outdated
Show resolved
Hide resolved
|
@Yohahaha left some comments here, and I suggest to simplify this title to |
wuchong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Yohahaha, I agree with @YannByron's comments. Could you please update the pull request to address them?
wuchong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yohahaha Thanks. I added a commit to add a test to verify altering a table.* config
| intercept[ExecutionException] { | ||
| sql( | ||
| s"ALTER TABLE t SET TBLPROPERTIES('${ConfigOptions.TABLE_DATALAKE_FORMAT.key()}' = 'paimon')") | ||
| }.getCause.shouldBe(a[InvalidAlterTableException]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this can be supported. But you need to enable datalake feature on the cluster.
Purpose
Linked issue: close #xxx
Brief change log
As title.
Tests
Catalog: set/remove table propertiesinorg.apache.fluss.spark.SparkCatalogTestAPI and Format
Documentation