[spark] Support ADD COLUMN in Spark catalog - #4174
Conversation
Yohahaha
left a comment
Yohahaha
left a comment
There was a problem hiding this comment.
left some comments, thank you!
| throw new UnsupportedOperationException( | ||
| s"Adding nested columns is not supported: ${p.fieldNames().mkString(".")}") | ||
| } | ||
| org.apache.fluss.metadata.TableChange.addColumn( |
There was a problem hiding this comment.
do we need support spark ColumnDefaultValue here?
There was a problem hiding this comment.
fluss doesn't support column defaults (no field anywhere in TableChange/Schema), and since we don't implement SupportsColumnDefaultValue
I added a guard, so a future SupportsColumnDefaultValue would fail loudly rather than drop defaults.
any case you think we should handle here?
|
|
||
| protected def lakeFormat: Option[DataLakeFormat] = None | ||
|
|
||
| test("Catalog: add columns") { |
There was a problem hiding this comment.
test not cover ColumnPosition, comments, nullable, please enrich test cases
Yohahaha
left a comment
Yohahaha
left a comment
There was a problem hiding this comment.
left some comments, please keep iterating tests coverage, thank you!
| // only the last position is supported: FIRST/AFTER fail at the Fluss RPC serialization | ||
| // layer (ColumnPositionType only knows LAST), surfaced by Spark as a SparkException |
There was a problem hiding this comment.
just throw exception in toFlussColumnPosition
| // fluss only supports adding nullable columns currently, the server rejects the change | ||
| // and the IllegalArgumentException is wrapped as UnknownServerException over the RPC |
There was a problem hiding this comment.
guard this behavior in conversion instead of test case
| // ALTER TABLE ADD COLUMN with a DEFAULT clause behaves differently: Spark silently drops | ||
| // the default value (the catalog does not support it) and still adds a nullable column at | ||
| // the end. | ||
| sql("ALTER TABLE t ADD COLUMN age bigint DEFAULT 18") |
There was a problem hiding this comment.
why this sql can success? does if (p.defaultValue() != null) check is fake?
2 participants
Purpose
Linked issue: close #xxx
support
alter table ... add columnin fluss-spark catalogBrief change log
AddColumnchanges to fluss table changes.Tests
added a test.
API and Format
no
Documentation
no