dtenedor commented on code in PR #59681:
URL: https://github.com/apache/spark/pull/59681#discussion_r4352179916
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/Resolver.scala:
##########
@@ -654,6 +657,13 @@ class Resolver(
val resolvedRelation = EvaluateUnresolvedInlineTable
.evaluateUnresolvedInlineTable(withResolvedExpressions)
+ resolvedRelation match {
Review Comment:
I tried omitting them, but dual-run mode is on in tests:
`spark.sql.analyzer.singlePassResolver.dualRunWithLegacy` defaults to
`Utils.isTesting`, and the sample rate is 1.0 in tests. Without these changes,
11 of the 21 tests in `InlineTableNonFoldableExpressionsSuite` and both
`inline-table.sql` golden tests failed with
`HYBRID_ANALYZER_EXCEPTION.SINGLE_PASS_FAILED_FIXED_POINT_SUCCEEDED`, because
the single-pass resolver can't handle the `Expand` or the scalar subquery. So I
kept them for now. Happy to handle this differently if you prefer.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/SubqueryExpressionResolver.scala:
##########
@@ -54,6 +55,9 @@ class SubqueryExpressionResolver(expressionResolver:
ExpressionResolver, resolve
*/
def resolveScalarSubquery(unresolvedScalarSubquery: ScalarSubquery):
Expression = {
traversals.current.parentOperator match {
+ case _: UnresolvedInlineTable
Review Comment:
Same here: without this case, the single-pass resolver throws
`SCALAR_SUBQUERY_IN_VALUES` for scalar subqueries in VALUES lists while the
fixed-point analyzer succeeds, so dual run fails those tests with
`SINGLE_PASS_FAILED_FIXED_POINT_SUCCEEDED`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/EvaluateUnresolvedInlineTable.scala:
##########
@@ -105,6 +141,28 @@ object EvaluateUnresolvedInlineTable extends SQLConfHelper
}
}
+ /**
+ * Returns whether the resolved expression can be evaluated by itself when
the query runs.
+ * Aggregate, window and generator functions are not meaningful in a VALUES
list. Outer
+ * references and subqueries other than uncorrelated scalar subqueries are
not supported, and
+ * neither are the remaining unevaluable expressions, such as SQL and Python
UDFs, which would
+ * have to be rewritten into separate operators.
+ */
+ private def isRuntimeEvaluable(e: Expression): Boolean = {
Review Comment:
Could you say a bit more about what you have in mind? If the idea is to
decide whether a cell can be evaluated by evaluating it during analysis, I
don't think that works here: `rand()` and `uuid()` throw until they are
initialized, subqueries and `current_user()` can't be evaluated yet, a UDF may
be expensive, `raise_error()` should fail when the query runs, and NULL is a
valid value. This function only checks the structure of the expression.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala:
##########
@@ -92,20 +92,23 @@ object EvalInlineTables extends Rule[LogicalPlan] with
CastSupport {
def eval(table: ResolvedInlineTable): LocalRelation = {
val newRows: Seq[InternalRow] =
- table.rows.map { row => InternalRow.fromSeq(row.map { e =>
- try {
- prepareForEval(e).eval()
- } catch {
- case NonFatal(ex) =>
- table.failAnalysis(
- errorClass =
"INVALID_INLINE_TABLE.FAILED_SQL_EXPRESSION_EVALUATION",
- messageParameters = Map("sqlExpr" -> toSQLExpr(e)),
- cause = ex)
- }})
- }
+ table.rows.map(row =>
InternalRow.fromSeq(row.map(evalExpression(table, _))))
LocalRelation(table.output, newRows)
}
+
+ /** Evaluates one of the expressions of the given inline table. */
+ def evalExpression(table: ResolvedInlineTable, e: Expression): Any = {
+ try {
+ prepareForEval(e).eval()
Review Comment:
Yes, I think so: it's the existing error handling of
`EvalInlineTables.eval`, moved unchanged so that `toExpand` can reuse it. It
reports evaluation failures of constant cells as
`INVALID_INLINE_TABLE.FAILED_SQL_EXPRESSION_EVALUATION`. Without it, `VALUES
(current_date(), 1 / 0)` with ANSI mode, for example, would fail with a raw
`DIVIDE_BY_ZERO` instead of today's error.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2796,6 +2796,10 @@ class Analyzer(
resolveSubQueries(j, j)
case tvf: UnresolvedTableValuedFunction =>
resolveSubQueries(tvf, tvf)
+ // An inline table has no output, so its subqueries cannot reference its
columns.
+ case t: UnresolvedInlineTable
Review Comment:
Fixed in a3a2849f2b1: `CheckAnalysis` only reports
`SCALAR_SUBQUERY_IN_VALUES` with the conf off. With the conf on, these queries
now fail with `UNRESOLVED_COLUMN` and `TABLE_OR_VIEW_NOT_FOUND`, in both
`SELECT` and `INSERT ... VALUES` (new test `unresolved references in VALUES
lists with scalar subqueries`). The conf-off error is unchanged.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/EvaluateUnresolvedInlineTable.scala:
##########
@@ -85,18 +120,19 @@ object EvaluateUnresolvedInlineTable extends SQLConfHelper
}
/**
- * Validates that all inline table data are valid expressions that can be
evaluated
- * (in this they must be foldable).
- * Note that nondeterministic expressions are not supported since they are
not foldable.
- * Exception are CURRENT_LIKE expressions, which are replaced by a literal
in later stages.
+ * Validates that all inline table data are valid expressions that can be
evaluated: during the
+ * analysis phase (see [[canEvalDuringAnalysis]]), or, if
+ * [[SQLConf.INLINE_TABLE_NON_FOLDABLE_EXPRESSIONS_ENABLED]] is true, when
the query runs (see
+ * [[isRuntimeEvaluable]]).
* This is package visible for unit testing.
*/
def validateInputEvaluable(table: UnresolvedInlineTable): Unit = {
+ val allowRuntimeEvaluation =
+ conf.getConf(SQLConf.INLINE_TABLE_NON_FOLDABLE_EXPRESSIONS_ENABLED)
table.rows.foreach { row =>
row.foreach { e =>
- if (e.containsPattern(CURRENT_LIKE)) {
- // Do nothing.
- } else if (!e.resolved || !trimAliases(prepareForEval(e)).foldable) {
+ if (!canEvalDuringAnalysis(e) &&
Review Comment:
Fixed in a3a2849f2b1: when a VALUES list becomes an `Expand`, all its
expressions other than constants must pass `isRuntimeEvaluable`, including the
ones with CURRENT_LIKE expressions. Aggregate, window and generator functions
over `current_date()` next to `rand()` now fail with
`CANNOT_EVALUATE_EXPRESSION_IN_INLINE_TABLE`. Lists that are evaluated during
analysis are unchanged.
--
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]
