dongjoon-hyun commented on code in PR #60035:
URL: https://github.com/apache/spark/pull/60035#discussion_r4451334039
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala:
##########
@@ -831,15 +831,26 @@ object TableOutputResolver extends SQLConfHelper with
Logging {
genExpr: Expression,
matched: Seq[NamedExpression],
conf: SQLConf): Expression = {
- genExpr.transform {
- case u @ UnresolvedAttribute(name +: nestedFields) =>
- matched.collectFirst {
- case alias: Alias if conf.resolver(alias.name, name) =>
- nestedFields.foldLeft(alias.child) { (child, field) =>
- UnresolvedExtractValue(child, Literal(field))
- }
- }.getOrElse(u)
+ def storedValue(nameParts: Seq[String]): Option[Expression] =
matched.collectFirst {
+ case alias: Alias if conf.resolver(alias.name, nameParts.head) =>
+ nameParts.tail.foldLeft(alias.child) { (child, field) =>
+ UnresolvedExtractValue(child, Literal(field))
+ }
}
+
+ // The parser turns every reference in a lambda function into a lambda
variable, so a lambda
+ // variable that no enclosing lambda function declares refers to a column.
+ def resolve(e: Expression, lambdaVars: Seq[String]): Expression = e match {
+ case u: UnresolvedAttribute => storedValue(u.nameParts).getOrElse(u)
+ case v: UnresolvedNamedLambdaVariable
+ if !lambdaVars.exists(conf.resolver(_, v.nameParts.head)) =>
+ storedValue(v.nameParts).getOrElse(v)
Review Comment:
Thank you. I switched to your suggestion: the analyzer now resolves the
generation expressions on top of `Project(storedCols, query)`, and
`resolveGenerationExprReferences` is removed. Your example, 100 rows with 2000
elements, now takes 40 ms locally, versus 2286 ms with the previous commit and
46 ms on master. `IDENTIFIER('i') * 10` now stores `50`, and I added a test for
it.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala:
##########
@@ -831,15 +831,26 @@ object TableOutputResolver extends SQLConfHelper with
Logging {
genExpr: Expression,
matched: Seq[NamedExpression],
conf: SQLConf): Expression = {
- genExpr.transform {
- case u @ UnresolvedAttribute(name +: nestedFields) =>
- matched.collectFirst {
- case alias: Alias if conf.resolver(alias.name, name) =>
- nestedFields.foldLeft(alias.child) { (child, field) =>
- UnresolvedExtractValue(child, Literal(field))
- }
- }.getOrElse(u)
+ def storedValue(nameParts: Seq[String]): Option[Expression] =
matched.collectFirst {
+ case alias: Alias if conf.resolver(alias.name, nameParts.head) =>
+ nameParts.tail.foldLeft(alias.child) { (child, field) =>
+ UnresolvedExtractValue(child, Literal(field))
+ }
}
+
+ // The parser turns every reference in a lambda function into a lambda
variable, so a lambda
+ // variable that no enclosing lambda function declares refers to a column.
+ def resolve(e: Expression, lambdaVars: Seq[String]): Expression = e match {
+ case u: UnresolvedAttribute => storedValue(u.nameParts).getOrElse(u)
+ case v: UnresolvedNamedLambdaVariable
+ if !lambdaVars.exists(conf.resolver(_, v.nameParts.head)) =>
Review Comment:
Thank you. This check is gone with the change above, since the analyzer
binds the names now. The two cases store `[51]` and `[10]`.
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/GeneratedColumnWriteSuite.scala:
##########
@@ -337,6 +337,23 @@ class GeneratedColumnWriteSuite extends QueryTest with
DatasourceV2SQLBase {
}
}
+ test("SPARK-60817: INSERT by name computes generated column from the stored
(post-cast) " +
+ "value in a lambda") {
+ val tblName = "my_tab"
+ withTable(s"testcat.$tblName") {
+ sql(s"""CREATE TABLE testcat.$tblName(
+ | a ARRAY<INT>,
+ | i INT,
+ | g1 ARRAY<INT> GENERATED ALWAYS AS (transform(a, x -> x + i *
10)),
+ | g2 ARRAY<ARRAY<INT>> GENERATED ALWAYS AS (transform(a, i ->
transform(a, x -> i)))
+ |) USING foo""".stripMargin)
+ // 5.7 is cast to INT (= 5) when stored in `i`. `g1` must read the
stored value (1 + 5 * 10),
+ // not the pre-cast value (1 + 5.7 * 10). In `g2`, `i` is the lambda
variable, not the column.
+ sql(s"INSERT INTO testcat.$tblName(a, i) SELECT array(1), 5.7D")
Review Comment:
Thank you. I added a test where `d` is declared after `g`, and updated that
sentence in the description.
--
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]
