You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srielau (via GitHub)" <gi...@apache.org> on 2024/04/28 18:02:57 UTC
[PR] [WIP][SPARK-48031] view evolution [spark]
srielau opened a new pull request, #46267:
URL: https://github.com/apache/spark/pull/46267
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for 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: 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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591547501
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -491,6 +510,9 @@ case class CatalogTable(
if (tableType == CatalogTableType.VIEW) {
viewText.foreach(map.put("View Text", _))
viewOriginalText.foreach(map.put("View Original Text", _))
+ if (SQLConf.get.viewSchemaBinding != "UNSUPPORTED") {
Review Comment:
Shall we read from the properties instead of getting it from the SQLConf?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2103503061
@cloud-fan @gengliangwang I have addressed all comments (except the "override" on the, which intelij and Wenchen appear to be of different opinions).
The testsuite has been split up into:
* Default behavior depending on config.
* 1 file per mode.
I don't see a good way to have one file with 4 distinct modes since they have sufficient syntax and test differences that I think "spaghetti" is more readable than trying to maximize shared text.
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591609868
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -491,6 +510,9 @@ case class CatalogTable(
if (tableType == CatalogTableType.VIEW) {
viewText.foreach(map.put("View Text", _))
viewOriginalText.foreach(map.put("View Original Text", _))
+ if (SQLConf.get.viewSchemaBinding != "UNSUPPORTED") {
Review Comment:
That would not allow us to switch the feature off for 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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595505686
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala:
##########
@@ -546,6 +551,13 @@ class SparkSqlAstBuilder extends AstBuilder {
val originalText = source(ctx.query)
assert(Option(originalText).isDefined,
"'originalText' must be provided to create permanent view")
+ val schemaBinding = visitSchemaBinding(ctx.schemaBinding(0))
+ val finalSchemaBinding =
+ if (schemaBinding == SchemaEvolution && userSpecifiedColumns.nonEmpty) {
+ SchemaTypeEvolution
Review Comment:
shall we fail in this case?
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591544808
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -401,6 +401,25 @@ case class CatalogTable(
)
}
+ /**
+ * Return the schema binding mode. Defaults to SchemaCompensation if not a view or an older
+ * version, unless the viewSchemaBinding config is set to UNSUPPORTED
+ */
+ def viewSchemaMode: ViewSchemaMode = {
+ if (SQLConf.get.viewSchemaBinding == "UNSUPPORTED") {
+ SchemaUnsupported
+ } else {
+ val schemaMode = properties.getOrElse(VIEW_SCHEMA_MODE, SchemaCompensation.toString)
Review Comment:
Shall we use `SQLConf.get.viewSchemaBinding` as the default value instead of `SchemaCompensation.toString`? If we change the default value of the SQL conf in the future, we may miss updating this one.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596870581
##########
sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala:
##########
@@ -224,6 +224,7 @@ abstract class BaseSessionStateBuilder(
TableCapabilityCheck +:
CommandCheck +:
CollationCheck +:
+ ViewSyncSchemaToMetaStore +:
Review Comment:
we should put this rule in `HiveSessionStateBuilder` as well
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591513685
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
+ "'COMPENSATION': Any supported casts. " +
+ "'TYPE EVOLUTION': Adopt type changes." +
+ "'EVOLUTION': Adopt names, types and arity." +
+ "'UNSUPPORTED': Disable the feature.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("BINDING", "COMPENSATION", "TYPE EVOLUTION", "EVOLUTION", "UNSUPPORTED"))
Review Comment:
Let's rename `TYPE EVOLUTION` to `TYPE_EVOLUTION`, so that it is consistent with other values such as `CodegenObjectFactoryMode`
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
+ "'COMPENSATION': Any supported casts. " +
+ "'TYPE EVOLUTION': Adopt type changes." +
+ "'EVOLUTION': Adopt names, types and arity." +
+ "'UNSUPPORTED': Disable the feature.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("BINDING", "COMPENSATION", "TYPE EVOLUTION", "EVOLUTION", "UNSUPPORTED"))
Review Comment:
Let's rename `TYPE EVOLUTION` to `TYPE_EVOLUTION`, so that it is consistent with other values such as the values of `CodegenObjectFactoryMode`
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595681121
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala:
##########
@@ -546,6 +551,13 @@ class SparkSqlAstBuilder extends AstBuilder {
val originalText = source(ctx.query)
assert(Option(originalText).isDefined,
"'originalText' must be provided to create permanent view")
+ val schemaBinding = visitSchemaBinding(ctx.schemaBinding(0))
+ val finalSchemaBinding =
+ if (schemaBinding == SchemaEvolution && userSpecifiedColumns.nonEmpty) {
+ SchemaTypeEvolution
Review Comment:
I have been going back and forth on this.
If we returned an error, that would preclude a future config default of EVOLUTION, as it would rquire all views with column list to use an explicit WITH TYPE EVOLUTION.
If we drive this further on might also argue that we should raise an error on EVOLUTION without SELECT * in it.
Basically, without SELECT *: EVOLUTION == TYPE EVOLUTION.
Even without all this, I think not returning an error is "nicer", it's not that there are many choices and we need the users help o decide what they want us to do.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595489391
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
Review Comment:
shall we have different defaults for:
1. existing views
2. newly created views without the `WITH SCHEMA` clause.
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591605730
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
+ "'COMPENSATION': Any supported casts. " +
+ "'TYPE EVOLUTION': Adopt type changes." +
+ "'EVOLUTION': Adopt names, types and arity." +
+ "'UNSUPPORTED': Disable the feature.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("BINDING", "COMPENSATION", "TYPE EVOLUTION", "EVOLUTION", "UNSUPPORTED"))
Review Comment:
If we do this, would we also need to change it in the grammar for CREATE VIEW?
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2096926666
> BINDING
The view will become invalid if the query column-list changes except for the following conditions:
The column-list includes a star clause, and there are additional columns. These additional columns are ignored.
The type of one or more columns changed in a way that allows them to be safely cast to the types using implicit casting rules.
This is the default behavior.
@srielau The PR description seems wrong. The default is `COMPENSATION`, right?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596861036
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,65 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, false, viewQuery)
+ if (metaData.viewSchemaMode == SchemaTypeEvolution ||
+ metaData.viewSchemaMode == SchemaEvolution) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ val viewFields = metaData.schema.fields
+ val viewQueryFields = viewQuery.schema.fields
+ val session = SparkSession.getActiveSession.get
+ val redoSignature =
+ viewSchemaMode == SchemaEvolution && viewFields.length != viewQueryFields.length
Review Comment:
what happens if the number of columns doesn't change but the column names are all different? shall we still redo signature?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595515280
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala:
##########
@@ -437,6 +493,21 @@ object ViewHelper extends SQLConfHelper with Logging {
}
}
+ /**
+ * Convert the viewSchemaMode to `properties`.
+ * If the mode is UNSUPPORTED do not store anything for backward compatibility.
+ */
+ private def viewSchemaModeToProps(viewSchemaMode: ViewSchemaMode): Map[String, String] = {
+ if (viewSchemaMode == SchemaUnsupported) {
+ Map.empty
+ } else {
+ val modeNameJson = JString(viewSchemaMode.toString)
Review Comment:
why do we use JSON format here? The mode name is just a simple string.
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2088685593
@cloud-fan @gengliangwang This is ready for an initial review.
Other than docs, the main open issue is NOT NULL behavior and COMMENTs.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596850227
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/view.scala:
##########
@@ -35,6 +35,56 @@ object EliminateView extends Rule[LogicalPlan] with CastSupport {
}
}
+/**
+ * ViewBindingMode is used to specify the expected schema binding mode when we want to create or
+ * replace a view in [[CreateViewStatement]].
+ */
+sealed trait ViewSchemaMode {
+ override val toString: String = getClass.getSimpleName.stripSuffix("$")
+}
+
+/**
+ * SchemaBinding means the view only tolerates minimal changes to the underlying schema.
+ * It can tolerate extra columns in SELECT * and upcast to more generic types.
+ */
+object SchemaBinding extends ViewSchemaMode {
+ override val toString: String = "BINDING"
Review Comment:
```suggestion
override def toString: String = "BINDING"
```
since the function body is just a string literal
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1597322015
##########
sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala:
##########
@@ -224,6 +224,7 @@ abstract class BaseSessionStateBuilder(
TableCapabilityCheck +:
CommandCheck +:
CollationCheck +:
+ ViewSyncSchemaToMetaStore +:
Review Comment:
There already (?)
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596850227
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/view.scala:
##########
@@ -35,6 +35,56 @@ object EliminateView extends Rule[LogicalPlan] with CastSupport {
}
}
+/**
+ * ViewBindingMode is used to specify the expected schema binding mode when we want to create or
+ * replace a view in [[CreateViewStatement]].
+ */
+sealed trait ViewSchemaMode {
+ override val toString: String = getClass.getSimpleName.stripSuffix("$")
+}
+
+/**
+ * SchemaBinding means the view only tolerates minimal changes to the underlying schema.
+ * It can tolerate extra columns in SELECT * and upcast to more generic types.
+ */
+object SchemaBinding extends ViewSchemaMode {
+ override val toString: String = "BINDING"
Review Comment:
```suggestion
override def toString: String = "BINDING"
```
since the function body is just a string literal
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595668170
##########
docs/sql-ref-syntax-ddl-create-view.md:
##########
@@ -61,6 +61,17 @@ CREATE [ OR REPLACE ] [ [ GLOBAL ] TEMPORARY ] VIEW [ IF NOT EXISTS ] view_ident
* `[ ( column_name [ COMMENT column_comment ], ... ) ]` to specify column-level comments.
* `[ COMMENT view_comment ]` to specify view-level comments.
* `[ TBLPROPERTIES ( property_name = property_value [ , ... ] ) ]` to add metadata key-value pairs.
+ * `[ WITH SCHEMA { BINDING | COMPENSATION | [ TYPE ] EVOLUTION } ]` to specify how the view reacts to schema changes
+
+ This clause is not supported for `TEMPORARY` views.
Review Comment:
Temporary views are governed by the config. If the feature is disabled its unchanged (BINDING ). Otherwise COMPENSATION (our new default).
I'll make sure to add a test.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595711075
##########
sql/core/src/test/resources/sql-tests/results/view-schema-binding.sql.out:
##########
@@ -0,0 +1,3081 @@
+-- Automatically generated by SQLQueryTestSuite
+-- !query
+SET spark.sql.ansi.enabled = true
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.ansi.enabled true
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = DISABLED
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode DISABLED
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 44,
+ "fragment" : "WITH SCHEMA BINDING"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA COMPENSATION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 49,
+ "fragment" : "WITH SCHEMA COMPENSATION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA TYPE EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 51,
+ "fragment" : "WITH SCHEMA TYPE EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 46,
+ "fragment" : "WITH SCHEMA EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT 1
+View Original Text SELECT 1
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [1]
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
+-- !query schema
+struct<namespace:string,tableName:string,isTemporary:boolean,information:string>
+-- !query output
+default v false Catalog: spark_catalog
+Database: default
+Table: v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type: VIEW
+View Text: SELECT 1
+View Original Text: SELECT 1
+View Catalog and Namespace: spark_catalog.default
+View Query Output Columns: [1]
+Schema: root
+ |-- 1: integer (nullable = false)
+
+
+-- !query
+DROP VIEW IF EXISTS v
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE TEMPORARY VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
+-- !query schema
+struct<namespace:string,tableName:string,isTemporary:boolean,information:string>
+-- !query output
+ v true Table: v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type: VIEW
+View Text: SELECT 1
+View Catalog and Namespace: spark_catalog.default
+View Query Output Columns: [1]
+Schema: root
+ |-- 1: integer (nullable = false)
+
+
+-- !query
+DROP VIEW IF EXISTS v
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 BIGINT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "CANNOT_UP_CAST_DATATYPE",
+ "sqlState" : "42846",
+ "messageParameters" : {
+ "details" : "The type path of the target object is:\n\nYou can either add an explicit cast to the input data or choose a higher precision type of the field in the target object",
+ "expression" : "spark_catalog.default.t.c1",
+ "sourceType" : "\"BIGINT\"",
+ "targetType" : "\"INT\""
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = COMPENSATION
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 BIGINT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "CANNOT_UP_CAST_DATATYPE",
+ "sqlState" : "42846",
+ "messageParameters" : {
+ "details" : "The type path of the target object is:\n\nYou can either add an explicit cast to the input data or choose a higher precision type of the field in the target object",
+ "expression" : "spark_catalog.default.t.c1",
+ "sourceType" : "\"BIGINT\"",
+ "targetType" : "\"INT\""
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT, c2 INT) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int,c2:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+c2 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1, c2]
+
+
+-- !query
+DROP TABLE t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "INCOMPATIBLE_VIEW_SCHEMA_CHANGE",
+ "sqlState" : "51024",
+ "messageParameters" : {
+ "actualCols" : "[]",
+ "colName" : "c2",
+ "expectedNum" : "1",
+ "suggestion" : "CREATE OR REPLACE VIEW spark_catalog.default.v AS SELECT * FROM t",
+ "viewName" : "`spark_catalog`.`default`.`v`"
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+c2 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1, c2]
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode
Review Comment:
Will rework the QA.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596873275
##########
sql/core/src/test/resources/sql-tests/results/view-schema-binding-config.sql.out:
##########
@@ -0,0 +1,794 @@
+-- Automatically generated by SQLQueryTestSuite
+-- !query
+SET spark.sql.viewSchemaBindingMode
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = EVOLUTION
+-- !query schema
+struct<>
+-- !query output
+java.lang.IllegalArgumentException
+The value of spark.sql.viewSchemaBindingMode should be one of COMPENSATION, DISABLED, but was EVOLUTION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = DISABLED
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode DISABLED
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 44,
+ "fragment" : "WITH SCHEMA BINDING"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA COMPENSATION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 49,
+ "fragment" : "WITH SCHEMA COMPENSATION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA TYPE EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 51,
+ "fragment" : "WITH SCHEMA TYPE EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 46,
+ "fragment" : "WITH SCHEMA EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT 1
+View Original Text SELECT 1
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [1]
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
Review Comment:
what extra information we want to get from `SHOW TABLE EXTENDED`?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #46267: [SPARK-48031][SQL] Support view schema evolution
URL: https://github.com/apache/spark/pull/46267
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591540905
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
Review Comment:
Also, 5 modes seem hard to use..Do we consider reducing it to 3?
--
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-48031] view evolution [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1588643743
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -1325,6 +1325,12 @@
],
"sqlState" : "54001"
},
+ "FEATURE_NOT_ENABLED" : {
+ "message" : [
+ "The feature <featureName> is not enable. Consider setting the config: <configKey> to <configValue>. To enable this capability."
Review Comment:
Typo: "not [enabled]"
Typo: "the config `<configKey>` to `<configValue>` to enable this" (remove `:` and `.`)
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -4481,6 +4487,11 @@
"Table <tableName> does not support <operation>. Please check the current catalog and namespace to make sure the qualified table name is expected, and also check the catalog implementation which is configured by \"spark.sql.catalog\"."
]
},
+ "TEMPORARY_VIEW_WITH_SCHEMA_BINDING_MODE" : {
+ "message" : [
+ "Temporary views cannot be created with the WITH SCHEMA clause. Recreate the temporary view when teh underlying schema changes, or use a persisted view."
Review Comment:
Typo: "when [the] underlying"
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591604947
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
Review Comment:
Reducing it to three is hard:
* UNSUPPORTED we need to switch of the feature, including dumping at DESCRIBE EXTENDED, and not storing anything fr new views.
* BINDING is the current default
* COMPENSATION is the new default we have been drifting towards (see changes to DECIMAL (done), and STRUCT (proposed)).
* That only leaves TYPE EVOLUTION vs EVOLUTION.
Slim pickings...
What we could do is, support only the first three for config.
So TYPE EVOLUTION and EVOLUTION is one you must state explicitly.
We could be even more aggressive and say: UNSUPPORTED and COMPENSATION are the only options in config.
Now, to the docs. The description on CREATE VIEW is more verbose. I could point there
The way we (don't) document configs really discourages being verbose. After all SET -v is pretty much 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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596869407
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,65 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, false, viewQuery)
+ if (metaData.viewSchemaMode == SchemaTypeEvolution ||
+ metaData.viewSchemaMode == SchemaEvolution) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ val viewFields = metaData.schema.fields
+ val viewQueryFields = viewQuery.schema.fields
+ val session = SparkSession.getActiveSession.get
+ val redoSignature =
+ viewSchemaMode == SchemaEvolution && viewFields.length != viewQueryFields.length
+ val fieldNames = viewQuery.schema.fieldNames
+ val newProperties = if (redoSignature) {
+ generateViewProperties(
+ metaData.properties,
+ session,
+ fieldNames,
+ fieldNames,
+ metaData.viewSchemaMode)
+ } else {
+ metaData.properties
+ }
+
+ val redo = redoSignature || viewFields.zipWithIndex.exists { case (field, index) =>
+ val planField = viewQueryFields(index)
+ (field.dataType != planField.dataType ||
+ field.nullable != planField.nullable ||
+ (viewSchemaMode == SchemaEvolution && (
Review Comment:
if we hit this condition, shall we also redo signature?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595672298
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/view.scala:
##########
@@ -35,6 +35,56 @@ object EliminateView extends Rule[LogicalPlan] with CastSupport {
}
}
+/**
+ * ViewBindingMode is used to specify the expected schema binding mode when we want to create or
+ * replace a view in [[CreateViewStatement]].
+ */
+sealed trait ViewSchemaMode {
+ override val toString: String = getClass.getSimpleName.stripSuffix("$")
Review Comment:
Intelij seems to disagree
<img width="553" alt="Screenshot 2024-05-09 at 9 13 37 AM" src="https://github.com/apache/spark/assets/3514644/4af555f4-742d-4a86-a605-02ba14ee129c">
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595534768
##########
sql/core/src/test/resources/sql-tests/results/view-schema-binding.sql.out:
##########
@@ -0,0 +1,3081 @@
+-- Automatically generated by SQLQueryTestSuite
+-- !query
+SET spark.sql.ansi.enabled = true
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.ansi.enabled true
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = DISABLED
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode DISABLED
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 44,
+ "fragment" : "WITH SCHEMA BINDING"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA COMPENSATION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 49,
+ "fragment" : "WITH SCHEMA COMPENSATION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA TYPE EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 51,
+ "fragment" : "WITH SCHEMA TYPE EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 46,
+ "fragment" : "WITH SCHEMA EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT 1
+View Original Text SELECT 1
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [1]
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
+-- !query schema
+struct<namespace:string,tableName:string,isTemporary:boolean,information:string>
+-- !query output
+default v false Catalog: spark_catalog
+Database: default
+Table: v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type: VIEW
+View Text: SELECT 1
+View Original Text: SELECT 1
+View Catalog and Namespace: spark_catalog.default
+View Query Output Columns: [1]
+Schema: root
+ |-- 1: integer (nullable = false)
+
+
+-- !query
+DROP VIEW IF EXISTS v
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE TEMPORARY VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
+-- !query schema
+struct<namespace:string,tableName:string,isTemporary:boolean,information:string>
+-- !query output
+ v true Table: v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type: VIEW
+View Text: SELECT 1
+View Catalog and Namespace: spark_catalog.default
+View Query Output Columns: [1]
+Schema: root
+ |-- 1: integer (nullable = false)
+
+
+-- !query
+DROP VIEW IF EXISTS v
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 BIGINT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "CANNOT_UP_CAST_DATATYPE",
+ "sqlState" : "42846",
+ "messageParameters" : {
+ "details" : "The type path of the target object is:\n\nYou can either add an explicit cast to the input data or choose a higher precision type of the field in the target object",
+ "expression" : "spark_catalog.default.t.c1",
+ "sourceType" : "\"BIGINT\"",
+ "targetType" : "\"INT\""
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = COMPENSATION
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 BIGINT NOT NULL) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "CANNOT_UP_CAST_DATATYPE",
+ "sqlState" : "42846",
+ "messageParameters" : {
+ "details" : "The type path of the target object is:\n\nYou can either add an explicit cast to the input data or choose a higher precision type of the field in the target object",
+ "expression" : "spark_catalog.default.t.c1",
+ "sourceType" : "\"BIGINT\"",
+ "targetType" : "\"INT\""
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1]
+
+
+-- !query
+DROP TABLE IF EXISTS t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT, c2 INT) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT * FROM t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<c1:int,c2:int>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+c2 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1, c2]
+
+
+-- !query
+DROP TABLE t
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+CREATE TABLE t(c1 INT) USING PARQUET
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+SELECT * FROM v
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "INCOMPATIBLE_VIEW_SCHEMA_CHANGE",
+ "sqlState" : "51024",
+ "messageParameters" : {
+ "actualCols" : "[]",
+ "colName" : "c2",
+ "expectedNum" : "1",
+ "suggestion" : "CREATE OR REPLACE VIEW spark_catalog.default.v AS SELECT * FROM t",
+ "viewName" : "`spark_catalog`.`default`.`v`"
+ }
+}
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+c1 int
+c2 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT * FROM t
+View Original Text SELECT * FROM t
+View Schema Mode BINDING
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [c1, c2]
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode
Review Comment:
why do we check the config value here? We already checked the default value at the beginning.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595689617
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala:
##########
@@ -437,6 +493,21 @@ object ViewHelper extends SQLConfHelper with Logging {
}
}
+ /**
+ * Convert the viewSchemaMode to `properties`.
+ * If the mode is UNSUPPORTED do not store anything for backward compatibility.
+ */
+ private def viewSchemaModeToProps(viewSchemaMode: ViewSchemaMode): Map[String, String] = {
+ if (viewSchemaMode == SchemaUnsupported) {
+ Map.empty
+ } else {
+ val modeNameJson = JString(viewSchemaMode.toString)
Review Comment:
Cargo cult...
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1594819220
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
Review Comment:
@gengliangwang @cloud-fan
I have reduced the config to DISABLED/COMPENSATION. That makes it de-factor a feature flag with the option to extend it to a session level default config of the need arises.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1598272715
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,65 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, false, viewQuery)
+ if (metaData.viewSchemaMode == SchemaTypeEvolution ||
+ metaData.viewSchemaMode == SchemaEvolution) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ val viewFields = metaData.schema.fields
+ val viewQueryFields = viewQuery.schema.fields
+ val session = SparkSession.getActiveSession.get
+ val redoSignature =
+ viewSchemaMode == SchemaEvolution && viewFields.length != viewQueryFields.length
Review Comment:
Yep, I had the test, but didn't catch the bad describe info.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596180917
##########
sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala:
##########
@@ -224,6 +224,7 @@ abstract class BaseSessionStateBuilder(
TableCapabilityCheck +:
CommandCheck +:
CollationCheck +:
+ SyncViewsCheck +:
Review Comment:
"ViewSyncSchemaToMetaStore" coming
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595698980
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,66 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object SyncViewsCheck extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, isTempView, viewQuery)
+ if (metaData.viewSchemaMode != SchemaUnsupported && !isTempView) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ if (viewSchemaMode == SchemaTypeEvolution || viewSchemaMode == SchemaEvolution) {
+ val viewFields = metaData.schema.fields
+ val planFields = plan.schema.fields
Review Comment:
I've replaced plan with viewQuery.
I think these are the same though, right? view() just passes the schema through
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595854690
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
If you prefer having 5 modes in the WITH SCHEMA clause, let's have 5 modes in the SQL Conf as well. Sorry for causing the misunderstanding.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596852480
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,87 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take the column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case other => throw SparkException.internalError("Unexpected ViewSchemaMode")
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
+ }
} else {
- _.toLowerCase(Locale.ROOT)
- }
- val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
- val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
- val viewDDL = buildViewDDL(metadata, isTempView)
-
- viewColumnNames.zip(metadata.schema).map { case (name, field) =>
- val normalizedName = normalizeColName(name)
- val count = nameToCounts(normalizedName)
- val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
- nameToCurrentOrdinal(normalizedName) = ordinal + 1
- val col = GetViewColumnByNameAndOrdinal(
- metadata.identifier.toString, name, ordinal, count, viewDDL)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
- }
- } else {
- // For view created by hive, the parsed view plan may have different output columns with
- // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
- // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
- metadata.schema.zipWithIndex.map { case (field, index) =>
- val col = GetColumnByOrdinal(index, field.dataType)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
+ // For view created by hive, the parsed view plan may have different output columns with
+ // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
+ // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
+ metadata.schema.zipWithIndex.map { case (field, index) =>
+ val col = GetColumnByOrdinal(index, field.dataType)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case other => throw SparkException.internalError("Unexpected ViewSchemaMode")
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
Review Comment:
nit: let's create a methond to do the cast, to avoid code duplication
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,87 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take the column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case other => throw SparkException.internalError("Unexpected ViewSchemaMode")
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
+ }
} else {
- _.toLowerCase(Locale.ROOT)
- }
- val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
- val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
- val viewDDL = buildViewDDL(metadata, isTempView)
-
- viewColumnNames.zip(metadata.schema).map { case (name, field) =>
- val normalizedName = normalizeColName(name)
- val count = nameToCounts(normalizedName)
- val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
- nameToCurrentOrdinal(normalizedName) = ordinal + 1
- val col = GetViewColumnByNameAndOrdinal(
- metadata.identifier.toString, name, ordinal, count, viewDDL)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
- }
- } else {
- // For view created by hive, the parsed view plan may have different output columns with
- // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
- // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
- metadata.schema.zipWithIndex.map { case (field, index) =>
- val col = GetColumnByOrdinal(index, field.dataType)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
+ // For view created by hive, the parsed view plan may have different output columns with
+ // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
+ // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
+ metadata.schema.zipWithIndex.map { case (field, index) =>
+ val col = GetColumnByOrdinal(index, field.dataType)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case other => throw SparkException.internalError("Unexpected ViewSchemaMode")
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
Review Comment:
nit: let's create a method to do the cast, to avoid code duplication
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595660959
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
We did, @gengliangwang raised the concern whether that is needlessly complex.
I have been splitting the baby by removing all but these two. having on/off config which is not a boolean.
If users demand it we can easily expand the config to more values, if not we can deprecate the the config and remove 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-48031][SQL] Support view schema evolution [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2103095339
+CC @shardulm94, @robreeves
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596169480
##########
sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala:
##########
@@ -224,6 +224,7 @@ abstract class BaseSessionStateBuilder(
TableCapabilityCheck +:
CommandCheck +:
CollationCheck +:
+ SyncViewsCheck +:
Review Comment:
Unfortunately we don't have an analyzer extension point to run at the very end of the analysis phase. We can add one later, but for now `CheckAnlysis` is the best place to do it. Mabybe we can rename this rule to indicate that it has side effects?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2107356801
@cloud-fan @gengliangwang All of @cloud-fan comments have been accepted and addressed.
Shall we merge 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-48031][SQL] Support view schema evolution [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1602870592
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala:
##########
@@ -742,7 +742,7 @@ abstract class SessionCatalogSuite extends AnalysisTest with Eventually {
private def getViewPlan(metadata: CatalogTable): LogicalPlan = {
import org.apache.spark.sql.catalyst.dsl.expressions._
val projectList = metadata.schema.map { field =>
- UpCast(
+ Cast(
Review Comment:
Seems like this breaks non-ANSI build. Made a followup: https://github.com/apache/spark/pull/46614
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595483982
##########
docs/sql-ref-syntax-ddl-create-view.md:
##########
@@ -61,6 +61,17 @@ CREATE [ OR REPLACE ] [ [ GLOBAL ] TEMPORARY ] VIEW [ IF NOT EXISTS ] view_ident
* `[ ( column_name [ COMMENT column_comment ], ... ) ]` to specify column-level comments.
* `[ COMMENT view_comment ]` to specify view-level comments.
* `[ TBLPROPERTIES ( property_name = property_value [ , ... ] ) ]` to add metadata key-value pairs.
+ * `[ WITH SCHEMA { BINDING | COMPENSATION | [ TYPE ] EVOLUTION } ]` to specify how the view reacts to schema changes
+
+ This clause is not supported for `TEMPORARY` views.
Review Comment:
Temp view and persistent view share the same view resolution code path, what should be the mode for temp view? Still BINDING as before?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595486032
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/view.scala:
##########
@@ -35,6 +35,56 @@ object EliminateView extends Rule[LogicalPlan] with CastSupport {
}
}
+/**
+ * ViewBindingMode is used to specify the expected schema binding mode when we want to create or
+ * replace a view in [[CreateViewStatement]].
+ */
+sealed trait ViewSchemaMode {
+ override val toString: String = getClass.getSimpleName.stripSuffix("$")
Review Comment:
We don't need this override since all subclasses of `ViewSchemaMode` overrides `toString`
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595853192
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
Shall we also support only `COMPENSATION` in the `WITH SCHEMA` clause? We should make them consistent.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595519795
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,66 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object SyncViewsCheck extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, isTempView, viewQuery)
+ if (metaData.viewSchemaMode != SchemaUnsupported && !isTempView) =>
Review Comment:
```suggestion
case View(metaData, isTempView, viewQuery)
if (metaData.viewSchemaMode != SchemaUnsupported && !isTempView) =>
```
```suggestion
case View(metaData, false, viewQuery)
if metaData.viewSchemaMode == SchemaTypeEvolution || metaData.viewSchemaMode == SchemaEvolution =>
```
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595668170
##########
docs/sql-ref-syntax-ddl-create-view.md:
##########
@@ -61,6 +61,17 @@ CREATE [ OR REPLACE ] [ [ GLOBAL ] TEMPORARY ] VIEW [ IF NOT EXISTS ] view_ident
* `[ ( column_name [ COMMENT column_comment ], ... ) ]` to specify column-level comments.
* `[ COMMENT view_comment ]` to specify view-level comments.
* `[ TBLPROPERTIES ( property_name = property_value [ , ... ] ) ]` to add metadata key-value pairs.
+ * `[ WITH SCHEMA { BINDING | COMPENSATION | [ TYPE ] EVOLUTION } ]` to specify how the view reacts to schema changes
+
+ This clause is not supported for `TEMPORARY` views.
Review Comment:
Temporary views are governed by the config. If the feature is disabled its unchanged (BINDING ). Otherwise COMPENSATION (our new default).
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2107718906
the k8s test failure is unrelated, 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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591562459
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1304,7 +1315,8 @@ case class CreateView(
originalText: Option[String],
query: LogicalPlan,
allowExisting: Boolean,
- replace: Boolean) extends BinaryCommand with CTEInChildren {
+ replace: Boolean,
+ viewSchemaMode: ViewSchemaMode) extends BinaryCommand with CTEInChildren {
Review Comment:
Why do we need to show `viewSchemaMode` in creating view? Shall we just show it for Alter View?
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1593035689
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1304,7 +1315,8 @@ case class CreateView(
originalText: Option[String],
query: LogicalPlan,
allowExisting: Boolean,
- replace: Boolean) extends BinaryCommand with CTEInChildren {
+ replace: Boolean,
+ viewSchemaMode: ViewSchemaMode) extends BinaryCommand with CTEInChildren {
Review Comment:
We need this
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591608406
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -401,6 +401,25 @@ case class CatalogTable(
)
}
+ /**
+ * Return the schema binding mode. Defaults to SchemaCompensation if not a view or an older
+ * version, unless the viewSchemaBinding config is set to UNSUPPORTED
+ */
+ def viewSchemaMode: ViewSchemaMode = {
+ if (SQLConf.get.viewSchemaBinding == "UNSUPPORTED") {
+ SchemaUnsupported
+ } else {
+ val schemaMode = properties.getOrElse(VIEW_SCHEMA_MODE, SchemaCompensation.toString)
Review Comment:
I was considering that, but the case here is a view which predates the feature.
So if we pick up the config we would have views which behavior changes based on a session config.
That seems weird, even more so if they can refresh themselves.
I'd rather keep the config to only affect CREATE VIEW.
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591510039
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,21 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING = buildConf("spark.sql.defaultViewSchemaBinding")
+ .doc("Control the default behavior of views when the underlying schema changes. " +
+ "Valid values are: " +
+ "'BINDING': Safe casts only. " +
Review Comment:
probably we need to explain the difference between the followings:
* Safe casts
* supported casts
* Adopt type changes
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1593023412
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -401,6 +401,25 @@ case class CatalogTable(
)
}
+ /**
+ * Return the schema binding mode. Defaults to SchemaCompensation if not a view or an older
+ * version, unless the viewSchemaBinding config is set to UNSUPPORTED
+ */
+ def viewSchemaMode: ViewSchemaMode = {
+ if (SQLConf.get.viewSchemaBinding == "UNSUPPORTED") {
+ SchemaUnsupported
+ } else {
+ val schemaMode = properties.getOrElse(VIEW_SCHEMA_MODE, SchemaCompensation.toString)
+ .stripSuffix("\"").stripPrefix("\"")
+ schemaMode match {
+ case SchemaBinding.toString => SchemaBinding
+ case SchemaEvolution.toString => SchemaEvolution
+ case SchemaTypeEvolution.toString => SchemaTypeEvolution
+ case SchemaCompensation.toString => SchemaCompensation
Review Comment:
Not sure what you want me to do here.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595513707
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala:
##########
@@ -318,6 +329,51 @@ case class AlterViewAsCommand(
}
}
+/**
+ * Alter a view with given schema binding. If the view name contains database prefix, this command
+ * will alter a permanent view matching the given name, or throw an exception if view not exist.
+ * Else, this command will try to alter a temporary view first, if view not exist, try permanent
+ * view next, if still not exist, throw an exception.
+ *
+ * @param name the name of this view.
+ * @param viewSchemaMode The new schema binding mode.
+ */
+case class AlterViewSchemaBindingCommand(name: TableIdentifier, viewSchemaMode: ViewSchemaMode)
+ extends LeafRunnableCommand {
+
+ import ViewHelper._
+
+ override def run(session: SparkSession): Seq[Row] = {
+ val isTemporary = session.sessionState.catalog.isTempView(name)
+ if (isTemporary) {
+ throw QueryCompilationErrors.cannotAlterTempViewWithSchemaBindingError()
+ }
+ alterPermanentView(session, viewSchemaMode)
+ Seq.empty[Row]
+ }
+
+ private def alterPermanentView(session: SparkSession, viewSchemaMode: ViewSchemaMode): Unit = {
+ val viewMeta = session.sessionState.catalog.getTableMetadata(name)
+
+ // Detect cyclic view reference on ALTER VIEW.
Review Comment:
we don't need to do this as the view query is not changed.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595660959
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
We did, @gengliangwang raised the question whether that is needlessly complex.
I have been splitting the baby by removing all but these two. having on/off config which is not a boolean.
If users demand it we can easily expand the config to more values, if not we can deprecate the the config and remove 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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595880193
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
These are two very different things.
One is the ability to choose a default, the other is the ability to make a view do what we want.
The more I think about this, the more I am convinced that the current behavior is the correct one.
We SHOULD be opinionated and we should keep "implied" behavior to a minimum.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595675243
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
Review Comment:
I was debating this. It would basically grandfather all existing views to an explicit WITH SCHEMA BINDING.
But, IMHO, this causes more grief than benefit.
We want existing views to use WITH SCHEMA COMPENSATION, so we are protected from future type resolution changes as the recent DECIMAL one.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1598332828
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,65 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object ViewSyncSchemaToMetaStore extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, false, viewQuery)
+ if (metaData.viewSchemaMode == SchemaTypeEvolution ||
+ metaData.viewSchemaMode == SchemaEvolution) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ val viewFields = metaData.schema.fields
+ val viewQueryFields = viewQuery.schema.fields
+ val session = SparkSession.getActiveSession.get
+ val redoSignature =
+ viewSchemaMode == SchemaEvolution && viewFields.length != viewQueryFields.length
+ val fieldNames = viewQuery.schema.fieldNames
+ val newProperties = if (redoSignature) {
+ generateViewProperties(
+ metaData.properties,
+ session,
+ fieldNames,
+ fieldNames,
+ metaData.viewSchemaMode)
+ } else {
+ metaData.properties
+ }
+
+ val redo = redoSignature || viewFields.zipWithIndex.exists { case (field, index) =>
+ val planField = viewQueryFields(index)
+ (field.dataType != planField.dataType ||
+ field.nullable != planField.nullable ||
+ (viewSchemaMode == SchemaEvolution && (
Review Comment:
Yes, If we need to refresh the view for any reason and we are in EVOLUTION we need to regen.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595529865
##########
sql/core/src/test/resources/sql-tests/inputs/view-schema-binding.sql:
##########
@@ -0,0 +1,429 @@
+-- This test suits check the WITH SCHEMA clause
+-- Make sure we use ANSI_MODE to catch casting errors
Review Comment:
shall we always use ansi cast regardless of the ansi config?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596849473
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/view.scala:
##########
@@ -35,6 +35,56 @@ object EliminateView extends Rule[LogicalPlan] with CastSupport {
}
}
+/**
+ * ViewBindingMode is used to specify the expected schema binding mode when we want to create or
+ * replace a view in [[CreateViewStatement]].
+ */
+sealed trait ViewSchemaMode {
+ override val toString: String = getClass.getSimpleName.stripSuffix("$")
Review Comment:
I mean remove this entire line, not just the keyword `override`
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596873275
##########
sql/core/src/test/resources/sql-tests/results/view-schema-binding-config.sql.out:
##########
@@ -0,0 +1,794 @@
+-- Automatically generated by SQLQueryTestSuite
+-- !query
+SET spark.sql.viewSchemaBindingMode
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode COMPENSATION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = EVOLUTION
+-- !query schema
+struct<>
+-- !query output
+java.lang.IllegalArgumentException
+The value of spark.sql.viewSchemaBindingMode should be one of COMPENSATION, DISABLED, but was EVOLUTION
+
+
+-- !query
+SET spark.sql.viewSchemaBindingMode = DISABLED
+-- !query schema
+struct<key:string,value:string>
+-- !query output
+spark.sql.viewSchemaBindingMode DISABLED
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA BINDING AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 44,
+ "fragment" : "WITH SCHEMA BINDING"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA COMPENSATION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 49,
+ "fragment" : "WITH SCHEMA COMPENSATION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA TYPE EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 51,
+ "fragment" : "WITH SCHEMA TYPE EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v WITH SCHEMA EVOLUTION AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+org.apache.spark.sql.catalyst.parser.ParseException
+{
+ "errorClass" : "FEATURE_NOT_ENABLED",
+ "sqlState" : "56038",
+ "messageParameters" : {
+ "configKey" : "spark.sql.viewSchemaBindingMode",
+ "configValue" : "COMPENSATION",
+ "featureName" : "VIEW ... WITH SCHEMA ..."
+ },
+ "queryContext" : [ {
+ "objectType" : "",
+ "objectName" : "",
+ "startIndex" : 26,
+ "stopIndex" : 46,
+ "fragment" : "WITH SCHEMA EVOLUTION"
+ } ]
+}
+
+
+-- !query
+CREATE OR REPLACE VIEW v AS SELECT 1
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESCRIBE EXTENDED v
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+1 int
+
+# Detailed Table Information
+Catalog spark_catalog
+Database default
+Table v
+Created Time [not included in comparison]
+Last Access [not included in comparison]
+Created By [not included in comparison]
+Type VIEW
+View Text SELECT 1
+View Original Text SELECT 1
+View Catalog and Namespace spark_catalog.default
+View Query Output Columns [1]
+
+
+-- !query
+SHOW TABLE EXTENDED LIKE 'v'
Review Comment:
what extra information we want to get from `SHOW TABLE EXTENDED`?
--
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-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #46267:
URL: https://github.com/apache/spark/pull/46267#issuecomment-2094939625
@cloud-fan @gengliangwang This is ready now. Please review.
Docs are included in this PR.
--
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] [WIP][SPARK-48031] view evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1585894014
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -1323,7 +1323,13 @@
"The statement, including potential SQL functions and referenced views, was too complex to parse.",
"To mitigate this error divide the statement into multiple, less complex chunks."
],
- "sqlState" : "54001"
+ "sqlState": "54001"
Review Comment:
```suggestion
"sqlState" : "54001"
```
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -1323,7 +1323,13 @@
"The statement, including potential SQL functions and referenced views, was too complex to parse.",
"To mitigate this error divide the statement into multiple, less complex chunks."
],
- "sqlState" : "54001"
+ "sqlState": "54001"
+ },
+ "FEATURE_NOT_ENABLED" : {
+ "message" : [
+ "The feature <featureName> is not enable. Consider setting the config: <configKey> to <configValue>. To enable this capability."
+ ],
+ "sqlState": "56038"
Review Comment:
```suggestion
"sqlState" : "56038"
```
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591551240
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -4996,6 +4996,60 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
query = plan(ctx.query))
}
+ /**
+ * Defined the schema binding mode during CREATE or ALTER VIEW.
+ * The method also accept a NULL context, in which case it will return the session default
+ *
+ * {{{
+ * WITH SCHEMA [ BINDING | COMPENSATION | TYPE EVOLUTION | EVOLUTION ]
+ * }}}
+ */
+ override def visitSchemaBinding(ctx: SchemaBindingContext): ViewSchemaMode = {
+ if (ctx == null) {
+ // No schema binding specified, return the session default
+ conf.viewSchemaBinding match {
+ case SchemaBinding.toString => SchemaBinding
+ case SchemaEvolution.toString => SchemaEvolution
+ case SchemaCompensation.toString => SchemaCompensation
+ case SchemaTypeEvolution.toString => SchemaTypeEvolution
+ case other => SchemaUnsupported
+ }
+ } else if (SQLConf.get.viewSchemaBinding == "UNSUPPORTED") {
Review Comment:
```suggestion
} else if (conf.viewSchemaBinding == "UNSUPPORTED") {
```
--
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-48031] view evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1591545095
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -401,6 +401,25 @@ case class CatalogTable(
)
}
+ /**
+ * Return the schema binding mode. Defaults to SchemaCompensation if not a view or an older
+ * version, unless the viewSchemaBinding config is set to UNSUPPORTED
+ */
+ def viewSchemaMode: ViewSchemaMode = {
+ if (SQLConf.get.viewSchemaBinding == "UNSUPPORTED") {
+ SchemaUnsupported
+ } else {
+ val schemaMode = properties.getOrElse(VIEW_SCHEMA_MODE, SchemaCompensation.toString)
+ .stripSuffix("\"").stripPrefix("\"")
+ schemaMode match {
+ case SchemaBinding.toString => SchemaBinding
+ case SchemaEvolution.toString => SchemaEvolution
+ case SchemaTypeEvolution.toString => SchemaTypeEvolution
+ case SchemaCompensation.toString => SchemaCompensation
Review Comment:
nit: let's handle the default case in the match here.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595479851
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -1700,6 +1700,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_BINDING_MODE = buildConf("spark.sql.viewSchemaBindingMode")
+ .doc("Set to DISABLE to disable the WITH SCHEMA clause for view DDL and suppress the line in " +
+ " DESCRIBE EXTENDED. The default, and only other value, is COMPENSATION. Views without " +
+ " WITH SCHEMA clause are defaulted to WITH SCHEMA COMPENSATION.")
+ .version("4.0.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .checkValues(Set("COMPENSATION", "DISABLED"))
Review Comment:
why don't we allow other modes to be the default?
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595484586
##########
docs/sql-ref-syntax-ddl-create-view.md:
##########
@@ -80,6 +91,10 @@ CREATE GLOBAL TEMPORARY VIEW IF NOT EXISTS subscribed_movies
AS SELECT mo.member_id, mb.full_name, mo.movie_title
FROM movies AS mo INNER JOIN members AS mb
ON mo.member_id = mb.id;
+
+-- Create a view filtering the `orders` table which will adjustto schema changes in `orders`.
Review Comment:
```suggestion
-- Create a view filtering the `orders` table which will adjust to schema changes in `orders`.
```
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595522181
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,66 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object SyncViewsCheck extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
+ plan.foreach {
+ case View(metaData, isTempView, viewQuery)
+ if (metaData.viewSchemaMode != SchemaUnsupported && !isTempView) =>
+ val viewSchemaMode = metaData.viewSchemaMode
+ if (viewSchemaMode == SchemaTypeEvolution || viewSchemaMode == SchemaEvolution) {
+ val viewFields = metaData.schema.fields
+ val planFields = plan.schema.fields
Review Comment:
why do we care `plan.schema`? It's an arbitrary plan that reads a view.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595683366
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala:
##########
@@ -318,6 +329,51 @@ case class AlterViewAsCommand(
}
}
+/**
+ * Alter a view with given schema binding. If the view name contains database prefix, this command
+ * will alter a permanent view matching the given name, or throw an exception if view not exist.
+ * Else, this command will try to alter a temporary view first, if view not exist, try permanent
+ * view next, if still not exist, throw an exception.
+ *
+ * @param name the name of this view.
+ * @param viewSchemaMode The new schema binding mode.
+ */
+case class AlterViewSchemaBindingCommand(name: TableIdentifier, viewSchemaMode: ViewSchemaMode)
+ extends LeafRunnableCommand {
+
+ import ViewHelper._
+
+ override def run(session: SparkSession): Seq[Row] = {
+ val isTemporary = session.sessionState.catalog.isTempView(name)
+ if (isTemporary) {
+ throw QueryCompilationErrors.cannotAlterTempViewWithSchemaBindingError()
+ }
+ alterPermanentView(session, viewSchemaMode)
+ Seq.empty[Row]
+ }
+
+ private def alterPermanentView(session: SparkSession, viewSchemaMode: ViewSchemaMode): Unit = {
+ val viewMeta = session.sessionState.catalog.getTableMetadata(name)
+
+ // Detect cyclic view reference on ALTER VIEW.
Review Comment:
Ah, I had removed the code, and left the comment
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596122256
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,73 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case SchemaEvolution => col
Review Comment:
schemaMode can't be `SchemaEvolution` here, right?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,73 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case SchemaEvolution => col
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
+ }
} else {
- _.toLowerCase(Locale.ROOT)
- }
- val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
- val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
- val viewDDL = buildViewDDL(metadata, isTempView)
-
- viewColumnNames.zip(metadata.schema).map { case (name, field) =>
- val normalizedName = normalizeColName(name)
- val count = nameToCounts(normalizedName)
- val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
- nameToCurrentOrdinal(normalizedName) = ordinal + 1
- val col = GetViewColumnByNameAndOrdinal(
- metadata.identifier.toString, name, ordinal, count, viewDDL)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
- }
- } else {
- // For view created by hive, the parsed view plan may have different output columns with
- // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
- // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
- metadata.schema.zipWithIndex.map { case (field, index) =>
- val col = GetColumnByOrdinal(index, field.dataType)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
+ // For view created by hive, the parsed view plan may have different output columns with
Review Comment:
why the schema mode doesn't affect the hive created view? Shall we add some comment about it?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -620,3 +621,65 @@ object CollationCheck extends (LogicalPlan => Unit) {
private def isCollationExpression(expression: Expression): Boolean =
expression.isInstanceOf[Collation] || expression.isInstanceOf[Collate]
}
+
+
+/**
+ * This rule checks for references to views WITH SCHEMA [TYPE] EVOLUTION and synchronizes the
+ * catalog if evolution was detected.
+ * It does so by walking the resolved plan looking for View operators for persisted views.
+ */
+object SyncViewsCheck extends (LogicalPlan => Unit) {
+ def apply(plan: LogicalPlan): Unit = {
Review Comment:
```suggestion
def apply(plan: LogicalPlan): Unit = {
```
##########
sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala:
##########
@@ -224,6 +224,7 @@ abstract class BaseSessionStateBuilder(
TableCapabilityCheck +:
CommandCheck +:
CollationCheck +:
+ SyncViewsCheck +:
Review Comment:
The check rules should not come with side effects. Ideally it should check the whole plan and throw exception if something is wrong. However, it seems that there is side effect in this new rule:
```
session.sessionState.catalog.alterTable(updatedViewMeta)
```
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596184739
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,73 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case SchemaEvolution => col
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
+ }
} else {
- _.toLowerCase(Locale.ROOT)
- }
- val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
- val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
- val viewDDL = buildViewDDL(metadata, isTempView)
-
- viewColumnNames.zip(metadata.schema).map { case (name, field) =>
- val normalizedName = normalizeColName(name)
- val count = nameToCounts(normalizedName)
- val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
- nameToCurrentOrdinal(normalizedName) = ordinal + 1
- val col = GetViewColumnByNameAndOrdinal(
- metadata.identifier.toString, name, ordinal, count, viewDDL)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
- }
- } else {
- // For view created by hive, the parsed view plan may have different output columns with
- // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
- // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
- metadata.schema.zipWithIndex.map { case (field, index) =>
- val col = GetColumnByOrdinal(index, field.dataType)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
+ // For view created by hive, the parsed view plan may have different output columns with
Review Comment:
Good catch! I didn't understand the code and somehow concluded it doesn't apply for Hive tables.
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1595468220
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -4481,6 +4487,11 @@
"Table <tableName> does not support <operation>. Please check the current catalog and namespace to make sure the qualified table name is expected, and also check the catalog implementation which is configured by \"spark.sql.catalog\"."
]
},
+ "TEMPORARY_VIEW_WITH_SCHEMA_BINDING_MODE" : {
+ "message" : [
+ "Temporary views cannot be created with the WITH SCHEMA clause. Recreate the temporary view when teh underlying schema changes, or use a persisted view."
Review Comment:
```suggestion
"Temporary views cannot be created with the WITH SCHEMA clause. Recreate the temporary view when the underlying schema changes, or use a persisted view."
```
--
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-48031][SQL] Support view schema evolution [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #46267:
URL: https://github.com/apache/spark/pull/46267#discussion_r1596184905
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -945,54 +945,73 @@ class SessionCatalog(
throw QueryCompilationErrors.invalidViewText(viewText, metadata.qualifiedName)
}
}
- val projectList = if (!isHiveCreatedView(metadata)) {
- val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
- // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
- // output is the same with the view output.
- metadata.schema.fieldNames.toImmutableArraySeq
- } else {
- assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
- metadata.viewQueryColumnNames
- }
+ val schemaMode = metadata.viewSchemaMode
+ if (schemaMode == SchemaEvolution) {
+ View(desc = metadata, isTempView = isTempView, child = parsedPlan)
+ } else {
+ val projectList = if (!isHiveCreatedView(metadata)) {
+ val viewColumnNames = if (metadata.viewQueryColumnNames.isEmpty) {
+ // For view created before Spark 2.2.0, the view text is already fully qualified, the plan
+ // output is the same with the view output.
+ metadata.schema.fieldNames.toImmutableArraySeq
+ } else {
+ assert(metadata.viewQueryColumnNames.length == metadata.schema.length)
+ metadata.viewQueryColumnNames
+ }
- // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
- // change after the view has been created. We need to add an extra SELECT to pick the columns
- // according to the recorded column names (to get the correct view column ordering and omit
- // the extra columns that we don't require), with UpCast (to make sure the type change is
- // safe) and Alias (to respect user-specified view column names) according to the view schema
- // in the catalog.
- // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
- // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
- // number of duplications, and pick the corresponding attribute by ordinal.
- val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
- val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
- identity
+ // For view queries like `SELECT * FROM t`, the schema of the referenced table/view may
+ // change after the view has been created. We need to add an extra SELECT to pick the
+ // columns according to the recorded column names (to get the correct view column ordering
+ // and omit the extra columns that we don't require), with UpCast (to make sure the type
+ // change is safe) and Alias (to respect user-specified view column names) according to the
+ // view schema in the catalog.
+ // Note that, the column names may have duplication, e.g. `CREATE VIEW v(x, y) AS
+ // SELECT 1 col, 2 col`. We need to make sure that the matching attributes have the same
+ // number of duplications, and pick the corresponding attribute by ordinal.
+ val viewConf = View.effectiveSQLConf(metadata.viewSQLConfigs, isTempView)
+ val normalizeColName: String => String = if (viewConf.caseSensitiveAnalysis) {
+ identity
+ } else {
+ _.toLowerCase(Locale.ROOT)
+ }
+ val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
+ val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
+ val viewDDL = buildViewDDL(metadata, isTempView)
+
+ viewColumnNames.zip(metadata.schema).map { case (name, field) =>
+ val normalizedName = normalizeColName(name)
+ val count = nameToCounts(normalizedName)
+ val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
+ nameToCurrentOrdinal(normalizedName) = ordinal + 1
+ val col = GetViewColumnByNameAndOrdinal(
+ metadata.identifier.toString, name, ordinal, count, viewDDL)
+ val cast = schemaMode match {
+ /*
+ ** For schema binding, we cast the column to the expected type using safe cast only.
+ ** For legacy behavior, we cast the column to the expected type using safe cast only.
+ ** For schema compensation, we cast the column to the expected type using any cast
+ * in ansi mode.
+ ** For schema (type) evolution, we take teh column as is.
+ */
+ case SchemaBinding => UpCast(col, field.dataType)
+ case SchemaUnsupported => UpCast(col, field.dataType)
+ case SchemaCompensation => Cast(col, field.dataType, ansiEnabled = true)
+ case SchemaTypeEvolution => col
+ case SchemaEvolution => col
+ }
+ Alias(cast, field.name)(explicitMetadata = Some(field.metadata))
+ }
} else {
- _.toLowerCase(Locale.ROOT)
- }
- val nameToCounts = viewColumnNames.groupBy(normalizeColName).transform((_, v) => v.length)
- val nameToCurrentOrdinal = scala.collection.mutable.HashMap.empty[String, Int]
- val viewDDL = buildViewDDL(metadata, isTempView)
-
- viewColumnNames.zip(metadata.schema).map { case (name, field) =>
- val normalizedName = normalizeColName(name)
- val count = nameToCounts(normalizedName)
- val ordinal = nameToCurrentOrdinal.getOrElse(normalizedName, 0)
- nameToCurrentOrdinal(normalizedName) = ordinal + 1
- val col = GetViewColumnByNameAndOrdinal(
- metadata.identifier.toString, name, ordinal, count, viewDDL)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
- }
- } else {
- // For view created by hive, the parsed view plan may have different output columns with
- // the schema stored in metadata. For example: `CREATE VIEW v AS SELECT 1 FROM t`
- // the schema in metadata will be `_c0` while the parsed view plan has column named `1`
- metadata.schema.zipWithIndex.map { case (field, index) =>
- val col = GetColumnByOrdinal(index, field.dataType)
- Alias(UpCast(col, field.dataType), field.name)(explicitMetadata = Some(field.metadata))
+ // For view created by hive, the parsed view plan may have different output columns with
Review Comment:
I've replicated the code above
--
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