[SPARK-42262][SQL] Table schema changes via V2SessionCatalog with HiveExternalCatalog #39826
Conversation
…ix/V2sessioncatlogAlterTable
|
CC @cloud-fan @rdblue thanks! |
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala
Outdated
Show resolved
Hide resolved
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala
Outdated
Show resolved
Hide resolved
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala
Outdated
Show resolved
Hide resolved
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalV2SessionCatalogSuite.scala
Outdated
Show resolved
Hide resolved
| // Not supported changes in alterTableDataSchema | ||
| !change.isInstanceOf[RenameColumn] && !change.isInstanceOf[DeleteColumn] |
There was a problem hiding this comment.
This seems awkward. In SessionCatalog I see there is a comment "not supporting dropping columns yet" (emphasis added), should we instead make the change to allow this within alterTableDataSchema? It seems that it can make this much cleaner and have a clear separation of schema changes from alterTableDataSchema vs other changes in alterTable.
There was a problem hiding this comment.
Or another somewhat related question, is the right approach to instead make HiveExternalCatalog.alterTable() support modifying the schema? It seems like this would be more in line with other implementations of alterTable. I see this comment was added as part of PR #14155:
* Note: As of now, this doesn't support altering table schema, partition column names and bucket
* specification. We will ignore them even if users do specify different values for these fields.
It's not clear to me if this was intentional, or something that was intended to be built on top of / fixed. @cloud-fan , thoughts here?
There was a problem hiding this comment.
I understand what you say about RenameColumn and DeleteColumn, I am not clear which is the best option, in this case as it is not supported I considered that it was better to keep the current behavior.
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
2 participants
What changes were proposed in this pull request?
In the case of
V2SessionCatalog.alterTable,SessionCatalog.alterTableis called for any type of modification. In caseHiveExternalCatalogis configured, thealterTablemethod does not support schema modification. For schema modifications it is necessary to usealterTableDataSchema. For this particular case, operations like adding columns or changing columns are not working.If we analyze the commands that perform column modifications in
SessionCatalog(AlterTableChangeColumnCommand,AlterTableAddColumnsCommand) they use thealterTableDataSchemamethod for schema modification and notalterTable.In this PR it is proposed that the modifications of the schema in
V2sessionCatalogare done through thealterTableDataSchemamethod of theSessionCatalog.Why are the changes needed?
To fix this bug
Does this PR introduce any user-facing change?
No
How was this patch tested?
A
HiveExternalV2SessionCatalogSuiteis introduced to launch the same tests asV2SessionCatalogTableSuitebut with the hive catalog configured. Despite the improvements introduced not everything is supported forHiveExternalCatalogso some tests are excluded.