You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Shubhambhusate (via GitHub)" <gi...@apache.org> on 2026/01/06 10:53:19 UTC
[PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Shubhambhusate opened a new pull request, #53691:
URL: https://github.com/apache/spark/pull/53691
### What changes were proposed in this pull request?
This PR fixes the DROP operator in pipe syntax (|>) to support qualified column names.
### Why are the changes needed?
Previously, the DROP operator in pipe syntax only accepted simple identifiers due to the use of identifierSeq in the grammar. This prevented users from dropping struct fields using qualified names
### Does this PR introduce _any_ user-facing change?
Yes. Users can now use qualified column names with the DROP pipe operator.
Before changes:
<img width="1028" height="646" alt="Screenshot 2026-01-06 at 4 19 51 PM" src="https://github.com/user-attachments/assets/cb0e7c12-e44a-435c-a002-97b0019c9a4b" />
After changes:
<img width="1028" height="462" alt="Screenshot 2026-01-06 at 4 18 15 PM" src="https://github.com/user-attachments/assets/f0d48598-be4a-4672-bb7f-3647270eac99" />
### How was this patch tested?
Test 1:
Updated SQL test cases in pipe-operators.sql with positive test for dropping struct fields using qualified names
Updated expected results in pipe-operators.sql.out and analyzer-results/pipe-operators.sql.out
Test 2:
spark.sql("""
| SELECT 2 AS lhs_a, 1 AS lhs_b
| |> AS lhs
| |> JOIN (VALUES (2, 1)) AS rhs(a, b)
| ON lhs.lhs_a = rhs.a
| |> Drop lhs.lhs_a
| """).show()
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "Shubhambhusate (via GitHub)" <gi...@apache.org>.
Shubhambhusate commented on code in PR #53691:
URL: https://github.com/apache/spark/pull/53691#discussion_r2711654679
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -6659,10 +6659,12 @@ class AstBuilder extends DataTypeAstBuilder
}.getOrElse(Option(ctx.SET).map { _ =>
visitOperatorPipeSet(ctx, left)
}.getOrElse(Option(ctx.DROP).map { _ =>
- val ids: Seq[String] = visitIdentifierSeq(ctx.identifierSeq())
+ val ids: Seq[Seq[String]] =
+ ctx.multipartIdentifierList().multipartIdentifier.asScala.toSeq.map(
+ _.parts.asScala.map(_.getText).toSeq)
Review Comment:
I have made the changes can you review it
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #53691:
URL: https://github.com/apache/spark/pull/53691#issuecomment-3780214581
Merging to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "Shubhambhusate (via GitHub)" <gi...@apache.org>.
Shubhambhusate commented on code in PR #53691:
URL: https://github.com/apache/spark/pull/53691#discussion_r2711656468
##########
sql/core/src/test/resources/sql-tests/inputs/pipe-operators.sql:
##########
@@ -401,17 +401,18 @@ table t
|> extend 1 as `x.y.z`
|> drop `x.y.z`;
+-- Dropping a struct field using qualified name.
+table st
+|> drop col.i1;
Review Comment:
Added more tests in the mentioned above
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #53691:
URL: https://github.com/apache/spark/pull/53691#issuecomment-3714247401
## JIRA Issue Information
=== New Feature SPARK-54914 ===
Summary: Pipe Syntax - Qualified Column Names in DROP Operator
Assignee: None
Status: Open
Affected: ["4.1.0"]
---
*This comment was automatically generated by GitHub Actions*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #53691:
URL: https://github.com/apache/spark/pull/53691#discussion_r2709725828
##########
sql/core/src/test/resources/sql-tests/inputs/pipe-operators.sql:
##########
@@ -401,17 +401,18 @@ table t
|> extend 1 as `x.y.z`
|> drop `x.y.z`;
+-- Dropping a struct field using qualified name.
+table st
+|> drop col.i1;
Review Comment:
Other testing ideas:
* Table alias qualified names as per the PR description: `... |> AS t |> DROP t.column`
* Multiple qualified columns in a single DROP: `table st |> DROP col.i1, col.i2`
* Multi-level nested structs: `... |> DROP outer.middle.inner`
* For `SELECT * FROM t |> AS col |> DROP col.x`, is `col.x` interpreted as dropping field `x` from struct column `col`, or dropping column `x` from table alias `col`? Let's add a test to cover it
* (Negative test) Invalid qualified names: attempting to drop a non-existent qualified path: `table st |> DROP col.nonexistent`
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -6659,10 +6659,12 @@ class AstBuilder extends DataTypeAstBuilder
}.getOrElse(Option(ctx.SET).map { _ =>
visitOperatorPipeSet(ctx, left)
}.getOrElse(Option(ctx.DROP).map { _ =>
- val ids: Seq[String] = visitIdentifierSeq(ctx.identifierSeq())
+ val ids: Seq[Seq[String]] =
+ ctx.multipartIdentifierList().multipartIdentifier.asScala.toSeq.map(
+ _.parts.asScala.map(_.getText).toSeq)
Review Comment:
can you use the existing `visitMultipartIdentifier` helper for this? For example:
```
val ids: Seq[Seq[String]] =
ctx.multipartIdentifierList()
.multipartIdentifier()
.asScala
.toSeq
.map(visitMultipartIdentifier)
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor closed pull request #53691: [SPARK-54914] [SQL] Fixing DROP operator in pipe syntax to support qualified column names
URL: https://github.com/apache/spark/pull/53691
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org