This is an automated email from the ASF dual-hosted git repository.
peter-toth pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 8e4c032b1fae [SPARK-49828][SQL] Make Column(expression) usable outside
of the org.apache.spark package
8e4c032b1fae is described below
commit 8e4c032b1fae29f426c38f5934739342e78e10f7
Author: Peter Toth <[email protected]>
AuthorDate: Wed Aug 5 11:13:53 2026 +0200
[SPARK-49828][SQL] Make Column(expression) usable outside of the
org.apache.spark package
### What changes were proposed in this pull request?
This makes the intended `Column(expression)` conversion usable from outside
the `org.apache.spark` package:
1. `object Column` (`sql/api`) becomes public. Every member of it is
already `private[spark]` or narrower, so no new member becomes visible -- the
object simply becomes nameable, which is all the extension in point 2 needs.
The two `apply` overloads that relied on the object's own visibility are now
marked `private[spark]` explicitly.
2. A named `DeveloperApi ClassicConversions.column(e: Expression): Column`
is added, for callers who would rather not rely on an implicit.
3. A new `ExpressionToColumnSuite` in package `test.org.apache.spark.sql`,
i.e. outside `org.apache.spark`, pins both entry points and the round trip.
### Why are the changes needed?
`ClassicConversions.ColumnConstructorExt` is already a public
`DeveloperApi`:
```scala
implicit class ColumnConstructorExt(val c: Column.type) {
def apply(e: Expression): Column = ExpressionUtils.column(e)
}
```
It is meant to let extension developers write `Column(expr)`, but it cannot
be used outside `org.apache.spark`, because `object Column` is itself
`private[spark]`. So `Column(expr)` does not even compile there:
```scala
package com.example.ext
// error: not found: value Column
val c = Column(Literal(1))
```
`ExpressionUtils`, `ExpressionColumnNode` and `ColumnNode` are package
private as well, and SPARK-49022 removed the public `new Column(expr:
Expression)` constructor, so there is currently no public Expression -> Column
path at all. The reverse direction is public and works fine
(`ColumnConversions.expression(col)`, `col.expr`), which makes the gap
asymmetric. Today the only workaround is to put the helper in a package under
`org.apache.spark`, which is what `mllib`'s `ml/stat/Summari [...]
This also makes an existing review comment on #48306 true: hvanhovell
suggested "You could call
`org.apache.spark.sql.classic.ClassicConversions.column` directly if you want
to avoid implicits" -- that method did not exist until now.
Prior art: #48306 by holdenk took the broader approach of exposing the
`ColumnNode` AST types (`ExpressionColumnNode`,
`ColumnNodeToExpressionConverter`) and moving `columnNodeSupport.scala` out of
`sql.internal`. It was approved by hvanhovell, then the approval was dismissed
over the package choice, and the stale bot closed it. Most of that diff is now
obsolete, since the file has meanwhile moved to `org.apache.spark.sql.classic`.
This PR intentionally stays narrower and leaves the A [...]
Three points reviewers may want to weigh in on:
- The named factory is deliberately on `object ClassicConversions` rather
than on the `ClassicConversions` trait. On the trait it shadows
`functions.column(colName: String)` for anyone who mixes the trait in, which
breaks existing call sites -- 6 of them in `sql/core`'s own tests, via
`QueryTest.testImplicits`. The trade-off is that a cross-version shim
implementing the trait does not inherit it.
- `import ClassicConversions._` combined with `import functions.column` is
now ambiguous. It is opt-in and a compile error rather than anything silent,
but happy to rename (`columnOf`, `toColumn`) if preferred; `column` was chosen
to mirror `ColumnConversions.expression`.
- `object Column` has no public members, so it shows up as an empty entry
in the generated API docs. I left it without an `since` tag, since the object
itself has existed since 4.0, just not publicly.
### Does this PR introduce _any_ user-facing change?
Yes, it adds public API. No existing behavior changes.
- `object Column` becomes public (no new members).
- `ClassicConversions.column(e: Expression): Column` is new, tagged `since
4.4.0`.
Extension developers outside `org.apache.spark` can now write either:
```scala
import org.apache.spark.sql.classic.ClassicConversions._
val c = Column(myExpression)
// or, without the implicit:
val c = ClassicConversions.column(myExpression)
```
### How was this patch tested?
New
`sql/core/src/test/scala/test/org/apache/spark/sql/ExpressionToColumnSuite.scala`.
Its package is outside `org.apache.spark`, so compiling it is as much of the
test as running it -- that is the piece #48306 lacked, which is how the
visibility gap went unnoticed in the first place.
- The suite fails to compile on unmodified `master` with `not found: value
Column` at both `Column(...)` sites, which is exactly the error a user hits.
- 3 tests pass with the change: both entry points, plus use in an actual
query.
- `ColumnExpressionSuite`, `JavaColumnExpressionSuite`, `DataFrameSuite`,
`DatasetSuite`, `JavaDatasetSuite` and the full `sql-api` test suite pass (~900
tests).
- MiMa is clean against `spark-sql_2.13:4.0.0` and
`spark-sql-api_2.13:4.0.0`, and `compile` is clean across all modules,
including `mllib`, which consumes the `Column(node)` overload.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
Closes #57759 from peter-toth/SPARK-49828-expr-to-column.
Authored-by: Peter Toth <[email protected]>
Signed-off-by: Peter Toth <[email protected]>
(cherry picked from commit 4194feb8391f26b9fc16ea7059797db7f346bad1)
Signed-off-by: Peter Toth <[email protected]>
---
.../main/scala/org/apache/spark/sql/Column.scala | 16 +++++--
.../org/apache/spark/sql/classic/conversions.scala | 16 ++++++-
.../apache/spark/sql/ExpressionToColumnSuite.scala | 49 ++++++++++++++++++++++
3 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/Column.scala
b/sql/api/src/main/scala/org/apache/spark/sql/Column.scala
index 56a9787db092..4befebd8418b 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/Column.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/Column.scala
@@ -19,7 +19,7 @@ package org.apache.spark.sql
import scala.jdk.CollectionConverters._
-import org.apache.spark.annotation.Stable
+import org.apache.spark.annotation.{DeveloperApi, Stable}
import org.apache.spark.internal.Logging
import org.apache.spark.internal.LogKeys.{LEFT_EXPR, RIGHT_EXPR}
import org.apache.spark.sql.catalyst.parser.DataTypeParser
@@ -30,11 +30,19 @@ import org.apache.spark.sql.internal.{ColumnNode,
TableValuedFunctionArgument}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[spark] object Column {
+/**
+ * The companion object is public so that the `Column` type can be referenced
as a value. This
+ * allows an implementation to add a `Column(expression)` factory through an
extension method. All
+ * of its members are internal to Spark.
+ *
+ * @since 4.4.0
+ */
+@DeveloperApi
+object Column {
- def apply(colName: String): Column = new Column(colName)
+ private[spark] def apply(colName: String): Column = new Column(colName)
- def apply(node: => ColumnNode): Column = withOrigin(new Column(node))
+ private[spark] def apply(node: => ColumnNode): Column = withOrigin(new
Column(node))
/**
* Invoke a function with an options map as its last argument. If there are
no options, its
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/classic/conversions.scala
b/sql/core/src/main/scala/org/apache/spark/sql/classic/conversions.scala
index 3cfdace73c45..fd5771778b27 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/classic/conversions.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/classic/conversions.scala
@@ -59,7 +59,21 @@ trait ClassicConversions {
}
@DeveloperApi
-object ClassicConversions extends ClassicConversions
+object ClassicConversions extends ClassicConversions {
+ /**
+ * Convert an [[Expression]] into a [[Column]]. This is the counterpart of
+ * [[ColumnConversions.expression]], for callers that would rather name the
conversion than rely
+ * on the implicit [[ClassicConversions.ColumnConstructorExt]].
+ *
+ * This is intentionally defined on the object rather than on the
[[ClassicConversions]] trait:
+ * on the trait it would shadow `functions.column(colName: String)` for
everyone who mixes the
+ * trait in, which is the trait's documented use case.
+ *
+ * @since 4.4.0
+ */
+ @DeveloperApi
+ def column(e: Expression): Column = ExpressionUtils.column(e)
+}
/**
* Conversions from a [[Column]] to an [[Expression]].
diff --git
a/sql/core/src/test/scala/test/org/apache/spark/sql/ExpressionToColumnSuite.scala
b/sql/core/src/test/scala/test/org/apache/spark/sql/ExpressionToColumnSuite.scala
new file mode 100644
index 000000000000..c03453b4856a
--- /dev/null
+++
b/sql/core/src/test/scala/test/org/apache/spark/sql/ExpressionToColumnSuite.scala
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package test.org.apache.spark.sql
+
+import org.apache.spark.sql.{Column, QueryTest, Row}
+import org.apache.spark.sql.catalyst.expressions.{Expression, Literal}
+import org.apache.spark.sql.classic.{ClassicConversions, ColumnConversions}
+import org.apache.spark.sql.classic.ClassicConversions._
+import org.apache.spark.sql.test.SharedSparkSession
+
+/**
+ * Tests the public Expression <-> Column conversions from a package outside of
+ * `org.apache.spark`, which is the only way to catch that a step of the path
is package-private.
+ * Compiling this suite is as much a part of the test as running it.
+ */
+class ExpressionToColumnSuite extends QueryTest with SharedSparkSession {
+
+ test("SPARK-49828: build a Column from an Expression via the Column
companion") {
+ val e: Expression = Literal(1)
+ val c: Column = Column(e)
+ assert(ColumnConversions.expression(c) == e)
+ }
+
+ test("SPARK-49828: build a Column from an Expression via
ClassicConversions.column") {
+ val e: Expression = Literal(1)
+ val c: Column = ClassicConversions.column(e)
+ assert(ColumnConversions.expression(c) == e)
+ }
+
+ test("SPARK-49828: a Column built from an Expression is usable in a query") {
+ val df = spark.range(2).select(Column(Literal(1)).as("one"))
+ checkAnswer(df, Seq(Row(1), Row(1)))
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
