stevomitric commented on code in PR #59226:
URL: https://github.com/apache/spark/pull/59226#discussion_r4230974314
##########
sql/core/src/test/resources/sql-tests/analyzer-results/sql-udf.sql.out:
##########
@@ -3959,6 +3959,94 @@ org.apache.spark.sql.AnalysisException
}
+-- !query
+DECLARE OR REPLACE VARIABLE foo3_5_col STRING DEFAULT 'a'
+-- !query analysis
+CreateVariable default(cast(a as string), sql=''a''), true
++- ResolvedIdentifier
org.apache.spark.sql.catalyst.analysis.FakeSystemCatalog$@xxxxxxxx,
session.foo3_5_col
+
+
+-- !query
+CREATE FUNCTION foo3_5d(x INT) RETURNS INT RETURN x + length(foo3_5_col)
+-- !query analysis
+org.apache.spark.sql.AnalysisException
+{
+ "errorClass" : "INVALID_TEMP_OBJ_REFERENCE",
Review Comment:
will probably need migration guides since some CREATE FUNCTION statements
that used to succeed now fail.
##########
sql/core/src/test/resources/sql-tests/inputs/sql-udf.sql:
##########
@@ -817,6 +817,22 @@ CREATE FUNCTION foo3_5d(x INT) RETURNS TABLE (a INT)
RETURN SELECT foo3_5a(x);
-- Expect error: permanent function cannot reference temporary view
CREATE FUNCTION foo3_5d(x INT) RETURNS INT RETURN (SELECT SUM(a) FROM t);
CREATE FUNCTION foo3_5d(x INT) RETURNS TABLE (a INT) RETURN SELECT a FROM t;
+-- Expect error: permanent function cannot reference a temporary variable
Review Comment:
It might be useful also to include (maybe not just golden file) tests that
cover:
- IDENTIFIER as a table name (FROM IDENTIFIER(tbl_var))
- qualified session.var names
- more than one variable
- the nested-function case
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/CreateSQLFunctionCommand.scala:
##########
@@ -385,7 +396,8 @@ case class CreateSQLFunctionCommand(
throw UserDefinedFunctionErrors.invalidTempFuncReference(
routineName = name.asMultipart, tempFuncName = funcName)
}
- val tempVars = ViewHelper.collectTemporaryVariables(analyzed)
+ val tempVars =
+ (ViewHelper.collectTemporaryVariables(analyzed) ++
referredTempVarsUnderIdentifier).distinct
Review Comment:
is `spark.sql.legacy.allowSessionVariableInPersistedView` honored for
functions?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -575,6 +575,24 @@ class Analyzer(
}
}
+ /**
+ * Like [[execute]], but also returns the temporary variables recorded via
IDENTIFIER clauses
+ * during this analysis
(`AnalysisContext.referredTempVariableNamesUnderIdentifier`).
+ *
+ * SQL function creation analyzes the body, inspects it, and only then runs
`checkAnalysis`, so
+ * it cannot go through
[[executeAndCheckReferredTempVariablesUnderIdentifier]]. It still needs
+ * the recorded variables, which [[execute]] discards with its analysis
context: a persistent
+ * function must reject them and a temporary one stores them in its metadata.
+ */
+ def executeWithReferredTempVariablesUnderIdentifier(
Review Comment:
if we have an older persistent f2 with IDENTIFIER(col) in its body, with col
declared, CREATE FUNCTION g(x) RETURN f2(x) now fails, naming col. is this
intended?
--
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]
