You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "szehon-ho (via GitHub)" <gi...@apache.org> on 2025/06/07 01:20:25 UTC
[PR] [SPARK-524167][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
szehon-ho opened a new pull request, #51107:
URL: https://github.com/apache/spark/pull/51107
### What changes were proposed in this pull request?
When a View is created, ex CREATE VIEW v (a1 INT, a2 STRING) AS select c1, c2, it needs to save both set of columns (alias and query output).
1. Alias is saved as View Schema (a1 INT, b2 STRING).
2. Query output is saved as Table property w/index (c1, 0) (c2, 1)
In the new Schema Evolution mode, we never allow alias, so view schema == view query output schema. Every time we detect the output view schema changes, we sync the view's schema with view query schema, keeping the invariant.
So we can simplify the update in Schema Evolution mode to not update the table properties, and instead rely on the View Schema all the time.
### Why are the changes needed?
View Schema evolution is a useful mode. However, it requires a lot of permissions on the user querying the view, because that user needs to update the View definition. It will simplify auth if we do not have to update the properties too (which could be admin-level properties), and reduce the update to only the schema.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing unit test
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2981353740
Also maybe I should put this behind a flag, for compatibility reason for previous 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2974880208
Shall we also skip generating the view query output table properties when creating the view? @szehon-ho
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2153267832
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
+ if (viewSchemaMode == SchemaEvolution) {
+ JArray(schema.map(_.name).map(JString).toList)
Review Comment:
synced offline, this is because viewQueryColumnNames is from the table properties
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2133422167
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -727,9 +716,16 @@ object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
}
SchemaUtils.checkColumnNameDuplication(fieldNames.toImmutableArraySeq,
session.sessionState.conf.resolver)
- val updatedViewMeta = metaData.copy(
- properties = newProperties,
- schema = newSchema)
+ val updatedViewMeta =
+ if (viewSchemaMode == SchemaEvolution) {
+ metaData.copy(
Review Comment:
Note: we already seem to use the ViewSchema directly in the table resolution for SchemaEvolution mode: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L991 iiuc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2133434982
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
Review Comment:
Note: This is for describe view query output.
We already seem to use the ViewSchema directly in the table resolution for SchemaEvolution mode: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L991 iiuc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2153267832
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
+ if (viewSchemaMode == SchemaEvolution) {
+ JArray(schema.map(_.name).map(JString).toList)
Review Comment:
synced offline, this is because viewQueryColumnNames is from the table properties.
This part of code is used in DESCRIBE
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2982583948
Thanks, merging to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #51107: [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode
URL: https://github.com/apache/spark/pull/51107
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2133422167
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -727,9 +716,16 @@ object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
}
SchemaUtils.checkColumnNameDuplication(fieldNames.toImmutableArraySeq,
session.sessionState.conf.resolver)
- val updatedViewMeta = metaData.copy(
- properties = newProperties,
- schema = newSchema)
+ val updatedViewMeta =
+ if (viewSchemaMode == SchemaEvolution) {
+ metaData.copy(
Review Comment:
Note: we already seem to use the ViewSchema directly in the table resolution for SchemaEvolution mode: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L991 iiuc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2982583799
> Documentation Generation:
/__w/_temp/0d08ee56-1341-4270-a8c6-156869cc0e28.sh: 1: python3.9: not found
Error: Process completed with exit code 127.
This is not related. Rebasing the master branch of your spark fork will fix 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2978494716
@cloud-fan @gengliangwang can you take another look? Thanks
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2974600656
@cloud-fan can you take a look?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2153143925
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
+ if (viewSchemaMode == SchemaEvolution) {
+ JArray(schema.map(_.name).map(JString).toList)
Review Comment:
I am a bit confused. As per the comment of `SchemaEvolution`
```
/**
* SchemaEvolution means the view will adopt changed column types and number of columns.
* This is a result of not having a column list and WITH EVOLUTION.
* Without an explicit column list the will also adopt changes to column names.
* In this mode the view will refresh its metastore data on reference to keep it up to day.
*/
```
should the schema be the viewQueryColumnNames instead?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2981905049
@szehon-ho let's also add test to verify the property for view is not changed after schema evolution.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #51107:
URL: https://github.com/apache/spark/pull/51107#issuecomment-2982193829
I think its a unrelated error?
```
Documentation Generation:
/__w/_temp/0d08ee56-1341-4270-a8c6-156869cc0e28.sh: 1: python3.9: not found
Error: Process completed with exit code 127.
```
unless i miss another error
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2133434982
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
Review Comment:
Note: we already seem to use the ViewSchema directly in the table resolution for SchemaEvolution mode: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L991 iiuc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52417][SQL] Simplify Table properties handling in View Schema Evolution Mode [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #51107:
URL: https://github.com/apache/spark/pull/51107#discussion_r2133434982
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -629,9 +629,15 @@ case class CatalogTable(
if (lastAccessTime <= 0) JString("UNKNOWN")
else JLong(lastAccessTime)
- val viewQueryOutputColumns: JValue =
- if (viewQueryColumnNames.nonEmpty) JArray(viewQueryColumnNames.map(JString).toList)
- else JNull
+ val viewQueryOutputColumns: JValue = {
Review Comment:
This is for describe view query output.
Note we already seem to use the ViewSchema directly in the view resolution for SchemaEvolution mode: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L991 iiuc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org