(spark) branch master updated: [SPARK-49828][SQL] Make Column(expression) usable outside of the org.apache.spark package

[pt...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

peter-toth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 4194feb8391f [SPARK-49828][SQL] Make Column(expression) usable outside of the org.apache.spark package
4194feb8391f is described below

commit 4194feb8391f26b9fc16ea7059797db7f346bad1
Author: Peter Toth <pe...@gmail.com>
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 <pe...@gmail.com>
    Signed-off-by: Peter Toth <pe...@gmail.com>
---
 .../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: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org