dtenedor commented on code in PR #48940: URL: https://github.com/apache/spark/pull/48940#discussion_r1857204664
########## sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out: ########## @@ -678,6 +678,156 @@ org.apache.spark.sql.AnalysisException } +-- !query +table t +|> set x = 1 +-- !query schema +struct<y:string,x:int> Review Comment: Good catch. I was using "SELECT * EXCEPT" and appending the new expression at the end to implement the behavior. It turns out this changed the column order, so I updated the `UnresolvedStarExcept` expression to support performing the replacement in the original column position instead. This expression is now named `UnresolvedStarExceptOrReplace`. Note that outside of SQL pipe operators, we could choose to use this to support "SELECT * EXCEPT REPLACE" syntax [1] in the future if we want. I am not implementing that now in the PR in the interest of keeping the PRs small and focused, but leaving a note here that we could do it in the future if desired. [1] https://stackoverflow.com/questions/73514343/databases-that-support-select-except-replace ########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala: ########## @@ -5978,7 +5981,49 @@ class AstBuilder extends DataTypeAstBuilder withQueryResultClauses(c, withSubqueryAlias(), forPipeOperators = true) }.getOrElse( visitOperatorPipeAggregate(ctx, left) - ))))))))) + )))))))))) + } + + private def visitOperatorPipeSet( + ctx: OperatorPipeRightSideContext, left: LogicalPlan): LogicalPlan = { + val (setIdentifiers: Seq[String], setTargets: Seq[Expression]) = + visitOperatorPipeSetAssignmentSeq(ctx.operatorPipeSetAssignmentSeq()) + var plan = left + val visitedSetIdentifiers = mutable.Set.empty[String] + setIdentifiers.zip(setTargets).foreach { + case (_, _: Alias) => + operationNotAllowed( + "SQL pipe syntax |> SET operator with an alias assigned with [AS] aliasName", ctx) + case (ident, target) => + // Check uniqueness of the assignment keys. + val checkKey = if (SQLConf.get.caseSensitiveAnalysis) { + ident.toLowerCase(Locale.ROOT) + } else { + ident + } + if (visitedSetIdentifiers(checkKey)) { + operationNotAllowed( + s"SQL pipe syntax |> SET operator with duplicate assignment key $ident", ctx) + } + visitedSetIdentifiers += checkKey + // Add an UnresolvedStarExcept to exclude the SET expression name from the relation and + // add the new SET expression to the projection list. + // Use a PipeSelect expression to make sure it does not contain any aggregate functions. + val projectList: Seq[NamedExpression] = + Seq(UnresolvedStarExcept(None, Seq(Seq(ident))), Review Comment: Good question; no. I added a test case to cover this, and a specific error message (rather than a generic "syntax error") to help advise the user. -- 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]
