cloud-fan commented on code in PR #58606:
URL: https://github.com/apache/spark/pull/58606#discussion_r4028292060
##########
sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql:
##########
@@ -468,3 +468,169 @@ SELECT EXTRACT(IDENTIFIER('YEAR') FROM DATE'2024-01-15');
SELECT TIMESTAMPADD(IDENTIFIER('YEAR'), 1, DATE'2024-01-15');
DROP SCHEMA identifier_clause_test_schema;
+
+-- A variable read only by a temporary view's IDENTIFIER clause is still a
variable the view refers
+-- to, so it stays resolvable when the stored view text is analyzed again.
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_var_name STRING DEFAULT
'identifier_var_target';
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_view AS
+SELECT * FROM IDENTIFIER(identifier_var_name);
+SELECT count(*) AS row_count FROM identifier_var_view;
+DROP VIEW identifier_var_view;
+DROP VIEW identifier_var_target;
+DROP TEMPORARY VARIABLE identifier_var_name;
+
+-- A permanent view must still reject a temporary variable read via an
IDENTIFIER clause.
+CREATE TEMPORARY VIEW identifier_perm_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_perm_name STRING DEFAULT
'identifier_perm_target';
+CREATE VIEW identifier_perm_view AS SELECT * FROM
IDENTIFIER(identifier_perm_name);
+DROP VIEW identifier_perm_target;
+DROP TEMPORARY VARIABLE identifier_perm_name;
+
+-- A permanent ALTER VIEW must also reject a temporary variable read via an
IDENTIFIER clause.
+-- The IDENTIFIER target is a permanent table so the variable is the only
temporary object.
+CREATE SCHEMA identifier_alter_schema;
+USE identifier_alter_schema;
+CREATE TABLE identifier_alter_target (c1 INT) USING parquet;
+DECLARE OR REPLACE VARIABLE identifier_alter_name STRING DEFAULT
'identifier_alter_target';
+CREATE VIEW identifier_alter_view AS SELECT 1 AS c1;
+ALTER VIEW identifier_alter_view AS SELECT * FROM
IDENTIFIER(identifier_alter_name);
+SELECT * FROM identifier_alter_view;
+DROP VIEW identifier_alter_view;
+DROP TABLE identifier_alter_target;
+DROP TEMPORARY VARIABLE identifier_alter_name;
+USE default;
+DROP SCHEMA identifier_alter_schema;
+
+-- ALTER VIEW whose TARGET is picked by a variable, with a body that has no
temp dependency, must
+-- still succeed: the variable is not a dependency of the view definition.
+CREATE SCHEMA ivt_schema;
+USE ivt_schema;
+CREATE VIEW ivt_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE ivt_name STRING DEFAULT 'ivt_target';
+ALTER VIEW IDENTIFIER(ivt_name) AS SELECT 2 AS c1;
+SELECT * FROM ivt_target;
+DROP VIEW ivt_target;
+DROP TEMPORARY VARIABLE ivt_name;
+USE default;
+DROP SCHEMA ivt_schema;
+
+-- A temporary view whose body reads a variable via an IDENTIFIER clause in
expression position
+-- (here as a column name). Unlike the table-position case above, this
identifier expression
+-- resolves to a bare variable reference, so recording it requires visiting
the root of the
+-- expression tree. If it is not recorded, the variable is missing from the
stored view text and
+-- reading the view back fails.
+DECLARE OR REPLACE VARIABLE identifier_expr_col STRING DEFAULT 'c1';
+CREATE OR REPLACE TEMPORARY VIEW identifier_expr_view AS
+SELECT IDENTIFIER(identifier_expr_col) AS x FROM VALUES(1) AS t(c1);
+SELECT * FROM identifier_expr_view;
+DROP VIEW identifier_expr_view;
+DROP TEMPORARY VARIABLE identifier_expr_col;
+
+-- A GLOBAL TEMPORARY VIEW reads a variable via an IDENTIFIER clause the same
way a session-local
+-- temporary view does, so the variable stays resolvable when the stored view
text is analyzed again.
+DECLARE OR REPLACE VARIABLE identifier_gtv_col STRING DEFAULT 'c1';
+CREATE OR REPLACE GLOBAL TEMPORARY VIEW identifier_gtv_view AS
+SELECT IDENTIFIER(identifier_gtv_col) AS x FROM VALUES(1) AS t(c1);
+SELECT * FROM global_temp.identifier_gtv_view;
+DROP VIEW global_temp.identifier_gtv_view;
+DROP TEMPORARY VARIABLE identifier_gtv_col;
+
+-- A variable used only to supply a view's NAME via an IDENTIFIER clause is
not part of the view
+-- definition, so it must not be recorded as a referred variable of the view.
+DECLARE OR REPLACE VARIABLE identifier_view_name STRING DEFAULT
'identifier_named_view';
+CREATE OR REPLACE TEMPORARY VIEW IDENTIFIER(identifier_view_name) AS SELECT 1
AS c1;
+SELECT * FROM identifier_named_view;
+DROP VIEW identifier_named_view;
+DROP TEMPORARY VARIABLE identifier_view_name;
+
+-- When both the NAME and the BODY use an IDENTIFIER clause, only the body
variable is a dependency
+-- of the view; the name variable must not be recorded.
+DECLARE OR REPLACE VARIABLE identifier_name_part STRING DEFAULT
'identifier_named_view2';
+DECLARE OR REPLACE VARIABLE identifier_body_part STRING DEFAULT 'c1';
+CREATE OR REPLACE TEMPORARY VIEW IDENTIFIER(identifier_name_part) AS
+SELECT IDENTIFIER(identifier_body_part) AS x FROM VALUES(1) AS t(c1);
+SELECT * FROM identifier_named_view2;
+DROP VIEW identifier_named_view2;
+DROP TEMPORARY VARIABLE identifier_name_part;
+DROP TEMPORARY VARIABLE identifier_body_part;
+
+-- ALTER VIEW honors `spark.sql.legacy.allowSessionVariableInPersistedView`
just like CREATE VIEW:
+-- with it set, the DDL is allowed instead of rejected (the legacy permissive
behavior). The flag
+-- does not persist the variable into the stored view, so the view is not
guaranteed to resolve in a
+-- fresh session; the default (tested by `identifier_alter_view` above)
rejects it outright.
+SET spark.sql.legacy.allowSessionVariableInPersistedView=true;
+DECLARE OR REPLACE VARIABLE identifier_p2a_col STRING DEFAULT 'c1';
+CREATE VIEW identifier_p2a_view AS SELECT 1 AS c1;
+ALTER VIEW identifier_p2a_view AS SELECT IDENTIFIER(identifier_p2a_col) FROM
VALUES(1) AS t(c1);
+DROP VIEW identifier_p2a_view;
+DROP TEMPORARY VARIABLE identifier_p2a_col;
+SET spark.sql.legacy.allowSessionVariableInPersistedView=false;
+
+-- Nested temporary views lock in the `withAnalysisContext` accumulator reset:
creating the outer
Review Comment:
Confirmed: the current analyzer-result coverage checks that the nested
dependency belongs to the inner view and is absent from the outer view.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4015151247","thread_id":"inline:4015151247","verdict_sha256":"f8b9e9e8019e17927c80e73ee028149c331391e2c415bc53aa6ac479a6ce91bb"}
-->
##########
sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql:
##########
@@ -468,3 +468,169 @@ SELECT EXTRACT(IDENTIFIER('YEAR') FROM DATE'2024-01-15');
SELECT TIMESTAMPADD(IDENTIFIER('YEAR'), 1, DATE'2024-01-15');
DROP SCHEMA identifier_clause_test_schema;
+
+-- A variable read only by a temporary view's IDENTIFIER clause is still a
variable the view refers
+-- to, so it stays resolvable when the stored view text is analyzed again.
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_var_name STRING DEFAULT
'identifier_var_target';
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_view AS
+SELECT * FROM IDENTIFIER(identifier_var_name);
+SELECT count(*) AS row_count FROM identifier_var_view;
+DROP VIEW identifier_var_view;
+DROP VIEW identifier_var_target;
+DROP TEMPORARY VARIABLE identifier_var_name;
+
+-- A permanent view must still reject a temporary variable read via an
IDENTIFIER clause.
+CREATE TEMPORARY VIEW identifier_perm_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_perm_name STRING DEFAULT
'identifier_perm_target';
+CREATE VIEW identifier_perm_view AS SELECT * FROM
IDENTIFIER(identifier_perm_name);
+DROP VIEW identifier_perm_target;
+DROP TEMPORARY VARIABLE identifier_perm_name;
+
+-- A permanent ALTER VIEW must also reject a temporary variable read via an
IDENTIFIER clause.
+-- The IDENTIFIER target is a permanent table so the variable is the only
temporary object.
+CREATE SCHEMA identifier_alter_schema;
+USE identifier_alter_schema;
+CREATE TABLE identifier_alter_target (c1 INT) USING parquet;
+DECLARE OR REPLACE VARIABLE identifier_alter_name STRING DEFAULT
'identifier_alter_target';
+CREATE VIEW identifier_alter_view AS SELECT 1 AS c1;
+ALTER VIEW identifier_alter_view AS SELECT * FROM
IDENTIFIER(identifier_alter_name);
+SELECT * FROM identifier_alter_view;
+DROP VIEW identifier_alter_view;
+DROP TABLE identifier_alter_target;
+DROP TEMPORARY VARIABLE identifier_alter_name;
+USE default;
+DROP SCHEMA identifier_alter_schema;
+
+-- ALTER VIEW whose TARGET is picked by a variable, with a body that has no
temp dependency, must
+-- still succeed: the variable is not a dependency of the view definition.
+CREATE SCHEMA ivt_schema;
+USE ivt_schema;
+CREATE VIEW ivt_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE ivt_name STRING DEFAULT 'ivt_target';
+ALTER VIEW IDENTIFIER(ivt_name) AS SELECT 2 AS c1;
+SELECT * FROM ivt_target;
+DROP VIEW ivt_target;
+DROP TEMPORARY VARIABLE ivt_name;
+USE default;
+DROP SCHEMA ivt_schema;
+
+-- A temporary view whose body reads a variable via an IDENTIFIER clause in
expression position
+-- (here as a column name). Unlike the table-position case above, this
identifier expression
+-- resolves to a bare variable reference, so recording it requires visiting
the root of the
+-- expression tree. If it is not recorded, the variable is missing from the
stored view text and
+-- reading the view back fails.
+DECLARE OR REPLACE VARIABLE identifier_expr_col STRING DEFAULT 'c1';
+CREATE OR REPLACE TEMPORARY VIEW identifier_expr_view AS
+SELECT IDENTIFIER(identifier_expr_col) AS x FROM VALUES(1) AS t(c1);
+SELECT * FROM identifier_expr_view;
+DROP VIEW identifier_expr_view;
+DROP TEMPORARY VARIABLE identifier_expr_col;
+
+-- A GLOBAL TEMPORARY VIEW reads a variable via an IDENTIFIER clause the same
way a session-local
+-- temporary view does, so the variable stays resolvable when the stored view
text is analyzed again.
+DECLARE OR REPLACE VARIABLE identifier_gtv_col STRING DEFAULT 'c1';
+CREATE OR REPLACE GLOBAL TEMPORARY VIEW identifier_gtv_view AS
+SELECT IDENTIFIER(identifier_gtv_col) AS x FROM VALUES(1) AS t(c1);
+SELECT * FROM global_temp.identifier_gtv_view;
+DROP VIEW global_temp.identifier_gtv_view;
+DROP TEMPORARY VARIABLE identifier_gtv_col;
+
+-- A variable used only to supply a view's NAME via an IDENTIFIER clause is
not part of the view
Review Comment:
Confirmed: the current suite exposes the temporary-view metadata for
target-only and mixed cases and adds a persisted target-only CREATE that would
fail if the target variable leaked.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4015151251","thread_id":"inline:4015151251","verdict_sha256":"f8b9e9e8019e17927c80e73ee028149c331391e2c415bc53aa6ac479a6ce91bb"}
-->
##########
sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql:
##########
@@ -468,3 +468,169 @@ SELECT EXTRACT(IDENTIFIER('YEAR') FROM DATE'2024-01-15');
SELECT TIMESTAMPADD(IDENTIFIER('YEAR'), 1, DATE'2024-01-15');
DROP SCHEMA identifier_clause_test_schema;
+
+-- A variable read only by a temporary view's IDENTIFIER clause is still a
variable the view refers
+-- to, so it stays resolvable when the stored view text is analyzed again.
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_var_name STRING DEFAULT
'identifier_var_target';
+CREATE OR REPLACE TEMPORARY VIEW identifier_var_view AS
+SELECT * FROM IDENTIFIER(identifier_var_name);
+SELECT count(*) AS row_count FROM identifier_var_view;
+DROP VIEW identifier_var_view;
+DROP VIEW identifier_var_target;
+DROP TEMPORARY VARIABLE identifier_var_name;
+
+-- A permanent view must still reject a temporary variable read via an
IDENTIFIER clause.
+CREATE TEMPORARY VIEW identifier_perm_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE identifier_perm_name STRING DEFAULT
'identifier_perm_target';
+CREATE VIEW identifier_perm_view AS SELECT * FROM
IDENTIFIER(identifier_perm_name);
+DROP VIEW identifier_perm_target;
+DROP TEMPORARY VARIABLE identifier_perm_name;
+
+-- A permanent ALTER VIEW must also reject a temporary variable read via an
IDENTIFIER clause.
+-- The IDENTIFIER target is a permanent table so the variable is the only
temporary object.
+CREATE SCHEMA identifier_alter_schema;
+USE identifier_alter_schema;
+CREATE TABLE identifier_alter_target (c1 INT) USING parquet;
+DECLARE OR REPLACE VARIABLE identifier_alter_name STRING DEFAULT
'identifier_alter_target';
+CREATE VIEW identifier_alter_view AS SELECT 1 AS c1;
+ALTER VIEW identifier_alter_view AS SELECT * FROM
IDENTIFIER(identifier_alter_name);
+SELECT * FROM identifier_alter_view;
+DROP VIEW identifier_alter_view;
+DROP TABLE identifier_alter_target;
+DROP TEMPORARY VARIABLE identifier_alter_name;
+USE default;
+DROP SCHEMA identifier_alter_schema;
+
+-- ALTER VIEW whose TARGET is picked by a variable, with a body that has no
temp dependency, must
+-- still succeed: the variable is not a dependency of the view definition.
+CREATE SCHEMA ivt_schema;
+USE ivt_schema;
+CREATE VIEW ivt_target AS SELECT 1 AS c1;
+DECLARE OR REPLACE VARIABLE ivt_name STRING DEFAULT 'ivt_target';
+ALTER VIEW IDENTIFIER(ivt_name) AS SELECT 2 AS c1;
+SELECT * FROM ivt_target;
+DROP VIEW ivt_target;
+DROP TEMPORARY VARIABLE ivt_name;
+USE default;
+DROP SCHEMA ivt_schema;
+
+-- A temporary view whose body reads a variable via an IDENTIFIER clause in
expression position
+-- (here as a column name). Unlike the table-position case above, this
identifier expression
+-- resolves to a bare variable reference, so recording it requires visiting
the root of the
+-- expression tree. If it is not recorded, the variable is missing from the
stored view text and
Review Comment:
Resolved in the current head: the comment now says the stored text retains
the IDENTIFIER reference and that the missing referred-variable metadata breaks
re-analysis.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4015151259","thread_id":"inline:4015151259","verdict_sha256":"f8b9e9e8019e17927c80e73ee028149c331391e2c415bc53aa6ac479a6ce91bb"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -166,6 +166,19 @@ case class AnalysisContext(
// lookup a temporary function. And export to the view metadata.
referredTempFunctionNames: mutable.Set[String] = mutable.Set.empty,
referredTempVariableNames: Seq[Seq[String]] = Seq.empty,
+ // Like `referredTempFunctionNames`, this is populated only by fixed-point
analysis (by
+ // `ResolveIdentifierClause`, the sole writer). A temporary view,
temporary ALTER VIEW, or
+ // CACHE TABLE AS SELECT stores the names in its metadata so they resolve
when the stored view
+ // text is analyzed again. A persisted CREATE/ALTER VIEW instead rejects
them -- usually in
+ // `ResolveIdentifierClause` while resolving the body, and otherwise (e.g.
an IDENTIFIER nested
+ // in a scalar subquery) in `verifyTemporaryObjectsNotExists` at run time.
When
Review Comment:
Confirmed: the current comment now distinguishes v1 execution-time
validation from the v2 `CheckViewReferences` analysis-time path.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4015151264","thread_id":"inline:4015151264","verdict_sha256":"f8b9e9e8019e17927c80e73ee028149c331391e2c415bc53aa6ac479a6ce91bb"}
-->
--
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]
