dongjoon-hyun opened a new pull request, #60040: URL: https://github.com/apache/spark/pull/60040
### What changes were proposed in this pull request? This PR makes `ALTER TABLE ... ADD CONSTRAINT ... CHECK (...)` fail with `EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE` when the target is a view. `UNIQUE`, `PRIMARY KEY`, and `FOREIGN KEY` already fail this way. `AddCheckConstraint` reads its target to validate the existing data. For that reason the parser gives it an `UnresolvedRelation` instead of the `UnresolvedTable` that the other constraint types use, and that relation can resolve to a view. `ResolveRelations` now rejects the target as soon as the relation is resolved: - `SubqueryAlias(_, View)` for a temporary, global temporary, or persistent view, including a view in a V2 catalog. - `MetricViewPlaceholder` for a metric view, which is not wrapped in a `SubqueryAlias`. The check is in `ResolveRelations` rather than `CheckAnalysis` because `ResolveSessionCatalog` runs before `CheckAnalysis` and fails on the table under the view. ### Why are the changes needed? Spark did not reject a view here, so the outcome depended on what the view reads. When the view only projects or filters a Data Source V2 table, the statement added an enforced constraint to that table: ```sql CREATE TABLE testcat.ns.t (id INT) USING foo; INSERT INTO testcat.ns.t VALUES (-1), (10); CREATE TEMPORARY VIEW tv AS SELECT * FROM testcat.ns.t WHERE id > 5; -- Issue 1: It should fail because `tv` is a view, but it succeeds and adds -- `CHECK (id > 0)` to `testcat.ns.t`, which has the row -1. ALTER TABLE tv ADD CONSTRAINT c CHECK (id > 0); ``` The constraint was validated only against the rows of the view. A view that renames a column caused a worse result: the table stored a condition on a column it does not have, and every later write to the table failed with `UNRESOLVED_COLUMN`. Other views failed with errors that did not describe the problem: | The view reads | Error before this PR | |---|---| | A V1 table | `UNSUPPORTED_FEATURE.TABLE_OPERATION` naming the underlying table | | A file path, through a DataFrame temporary view | `java.util.NoSuchElementException: None.get` | | Anything else, such as `range()` or a join | `INTERNAL_ERROR` (`No plan for AddCheckConstraint`) | | A V2 table, through a metric view | `INTERNAL_ERROR` (`No plan for AddCheckConstraint`) | ### Does this PR introduce _any_ user-facing change? Yes. All of the statements above now fail with `EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE`: ``` [EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE] 'ALTER TABLE ... ADD CONSTRAINT' expects a table but `tv` is a view. SQLSTATE: 42809 ``` This is a change compared to the released Spark 4.1, 4.2, and 4.3, which have the same parser and planner code. A statement that used to succeed on a view over a Data Source V2 table now fails, so this PR adds an entry to the SQL migration guide. Constraints that were already added this way stay on the table. If a view's definition no longer resolves, for example because its table was dropped, `CHECK` still reports `TABLE_OR_VIEW_NOT_FOUND`. This is because the view body is resolved before the target is checked. ### How was this patch tested? Pass the CIs with the newly added test cases in `CheckConstraintSuite`. Without this change, the view test fails because no exception is thrown, and the metric view test fails with `INTERNAL_ERROR`. ``` build/sbt 'sql/testOnly *command.v2.CheckConstraintSuite' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5.5 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
