You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hvanhovell (via GitHub)" <gi...@apache.org> on 2024/08/09 13:50:27 UTC
[PR] [WIP][SPARK-49022] Use Column Node API in Column [spark]
hvanhovell opened a new pull request, #47688:
URL: https://github.com/apache/spark/pull/47688
### What changes were proposed in this pull request?
TBD
### Why are the changes needed?
TBD
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No
### 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-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2295333950
@EnricoMi what are you exactly doing? Could you share an example? We can definitely open up the interface to allow for user extensions.
If it is basically adding functions, then I'd probably go with the `Column.fn(...)` approach in combination with a SparkSessionExtension that will register your custom expressions. This way we can easily make what you have created work with Spark Connect (and with SQL). I understand some folks are not interested in this, in that case we can still open up an expression way of creating a Column.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719253652
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
Review Comment:
Wonder if we should have a different name from `Wrapper` (with some scaladoc for that class). To me, it's a bit difficult to follow why we have `Wrapper` and what it dose.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719244876
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18628,15 +18623,10 @@ def _create_lambda(f: Callable) -> Callable:
parameters = _get_lambda_parameters(f)
sc = _get_active_spark_context()
- expressions = cast(JVMView, sc._jvm).org.apache.spark.sql.catalyst.expressions
+ internal = cast(JVMView, sc._jvm).org.apache.spark.sql.internal
Review Comment:
Seems not used
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719256960
##########
python/pyspark/sql/tests/test_dataframe.py:
##########
@@ -114,7 +114,6 @@ def test_count_star(self):
self.assertEqual(df3.select(count(df3["*"])).columns, ["count(1)"])
self.assertEqual(df3.select(count(col("*"))).columns, ["count(1)"])
- self.assertEqual(df3.select(count(col("s.*"))).columns, ["count(1)"])
Review Comment:
Because it is wrong. Or at least inconsistent with the the sql frontend.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719266892
##########
sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -26,8 +26,7 @@ import org.apache.spark.sql.catalyst.ScalaReflection
import org.apache.spark.sql.catalyst.encoders.AgnosticEncoders.UnboundRowEncoder
import org.apache.spark.sql.catalyst.encoders.encoderFor
import org.apache.spark.sql.catalyst.expressions.{Expression, ScalaUDF}
-import org.apache.spark.sql.execution.aggregate.ScalaAggregator
-import org.apache.spark.sql.internal.UserDefinedFunctionLike
+import org.apache.spark.sql.internal.{InvokeInlineUserDefinedFunction, UserDefinedFunctionLike}
Review Comment:
not so related to this PR but I wonder if we should name the package a little bit differently, e.g., `org.apache.spark.sql.internal.api`. At least I am checking which package does individual class belongs to.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2287643185
This is waiting for https://github.com/apache/spark/pull/47746.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720319265
##########
sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -26,8 +26,7 @@ import org.apache.spark.sql.catalyst.ScalaReflection
import org.apache.spark.sql.catalyst.encoders.AgnosticEncoders.UnboundRowEncoder
import org.apache.spark.sql.catalyst.encoders.encoderFor
import org.apache.spark.sql.catalyst.expressions.{Expression, ScalaUDF}
-import org.apache.spark.sql.execution.aggregate.ScalaAggregator
-import org.apache.spark.sql.internal.UserDefinedFunctionLike
+import org.apache.spark.sql.internal.{InvokeInlineUserDefinedFunction, UserDefinedFunctionLike}
Review Comment:
You mean move all ColumnNode classes elsewhere?
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719257971
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
Review Comment:
Another way is to add sth like `Expression.toNode` vs `ColumnNode.toExpr`
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719267743
##########
sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -212,3 +183,25 @@ private[sql] case class UserDefinedAggregator[IN, BUF, OUT](
override def name: String = givenName.getOrElse(aggregator.name)
}
+
+private[sql] object UserDefinedFunctionUtils {
Review Comment:
Should we name it `UserDefinedFunction` as a companion object?
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719255232
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -283,14 +279,18 @@ class Column(val expr: Expression) extends Logging {
* @since 1.3.0
*/
def ===(other: Any): Column = {
- val right = lit(other).expr
- if (this.expr == right) {
+ val right = lit(other)
+ checkTrivialPredicate(right)
+ fn("=", other)
+ }
+
+ private def checkTrivialPredicate(right: Column): Unit = {
+ if (this == right) {
logWarning(
log"Constructing trivially true equals predicate, " +
- log"'${MDC(LEFT_EXPR, this.expr)} = ${MDC(RIGHT_EXPR, right)}'. " +
+ log"'${MDC(LEFT_EXPR, this)} <=> ${MDC(RIGHT_EXPR, right)}'. " +
Review Comment:
```suggestion
log"'${MDC(LEFT_EXPR, this)} = ${MDC(RIGHT_EXPR, right)}'. " +
```
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719260320
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1590,6 +1590,8 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
} else {
a.copy(aggregateExpressions = buildExpandedProjectList(a.aggregateExpressions, a.child))
}
+ case c: CollectMetrics if containsStar(c.metrics) =>
+ c.copy(metrics = buildExpandedProjectList(c.metrics, c.child))
Review Comment:
I moved some of the hard coded star handling into the analyzer. For collect metrics we now have to support star expansion.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719250714
##########
python/pyspark/sql/tests/test_dataframe.py:
##########
@@ -114,7 +114,6 @@ def test_count_star(self):
self.assertEqual(df3.select(count(df3["*"])).columns, ["count(1)"])
self.assertEqual(df3.select(count(col("*"))).columns, ["count(1)"])
- self.assertEqual(df3.select(count(col("s.*"))).columns, ["count(1)"])
Review Comment:
Why did we remove this?
--
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-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2295438535
> backward compatibility
Which backward compatibility? Showing an example would help a lot to understand
--
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-49022] Use Column Node API in Column [spark]
Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2296226981
The [spark-extension](https://github.com/G-Research/spark-extension) packages provides some [Dataset diff tooling](https://github.com/G-Research/spark-extension/blob/master/DIFF.md). There, a user-defined comparison can simply be defined by implementing the [`scala.math.Equiv` interface](https://www.scala-lang.org/api/2.13.5/scala/math/Equiv.html): https://github.com/G-Research/spark-extension/blob/master/src/main/scala/uk/co/gresearch/spark/diff/DiffComparators.scala#L41
That `Equiv` implementation is wrapped into an `Expression` (including codegen) and turned into a `Comparator`that is then used by the package to diff columns: given two columns `left` and `right`, return a `Column` that evaluates (compares the columns) to `Boolean`:
- [EquivDiffComparator.scala:34](https://github.com/G-Research/spark-extension/blob/master/src/main/scala/uk/co/gresearch/spark/diff/comparator/EquivDiffComparator.scala#L34)
- [EquivDiffComparator.scala:67](https://github.com/G-Research/spark-extension/blob/master/src/main/scala/uk/co/gresearch/spark/diff/comparator/EquivDiffComparator.scala#L67)
This obviously won't work for Spark connect, but with Column Node API this does not work for classic Spark client either.
That package supports Spark 3.0 - 3.5. Creating a `Column` from an `Expression` would allow for minimal changes to keep this working for Spark 4.0 with non-Connect client. This is what I meant with backward compatibility.
In order to support Spark Connect, there is no way around using the Spark Connect plugin / extensions.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719250373
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18787,7 +18771,7 @@ def exists(col: "ColumnOrName", f: Callable[[Column], Column]) -> Column:
| true|
+------------+
"""
- return _invoke_higher_order_function("ArrayExists", [col], [f])
+ return _invoke_higher_order_function("exists", [col], [f])
Review Comment:
That is also a problem for all other functions in here.
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18628,15 +18623,10 @@ def _create_lambda(f: Callable) -> Callable:
parameters = _get_lambda_parameters(f)
sc = _get_active_spark_context()
- expressions = cast(JVMView, sc._jvm).org.apache.spark.sql.catalyst.expressions
+ internal = cast(JVMView, sc._jvm).org.apache.spark.sql.internal
Review Comment:
Yeah removing 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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719271705
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
- def unapply(col: Column): Option[Expression] = Some(col.expr)
+ def apply(node: => ColumnNode): Column = withOrigin(new Column(node))
Review Comment:
I think the error report here would be a bit weird via `Origin` because the top level of the function call changed. We get `Thread.currentThread().getStackTrace`, and set it to `Origin` so whichever function fails, it will always point it here.
Would be great if we can double check the code like the below still works fine:
```scala
val df = spark.range(10)
val df1 = df.withColumn("div_ten", df.col("id") / 10)
val df2 = df1.withColumn("plus_four", df.col("id") + 4)
// This is problematic divide operation that occurs DIVIDE_BY_ZERO.
val df3 = df2.withColumn("div_zero", df.col("id") / 0) // Error here
val df4 = df3.withColumn("minus_five", df.col("id") / 5)
df4.collect()
```
should report sth like:
```
org.apache.spark.SparkArithmeticException: [DIVIDE_BY_ZERO] Division by zero. Use `try_divide` to tolerate divisor being 0 and return NULL instead. If necessary set "spark.sql.ansi.enabled" to "false" to bypass this error. SQLSTATE: 22012
== DataFrame ==
"div" was called from
<init>(<console>:7)
```
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1716258329
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18669,18 +18665,12 @@ def _invoke_higher_order_function(
:return: a Column
"""
- from py4j.java_gateway import JVMView
- from pyspark.sql.classic.column import _to_java_column
+ from pyspark.sql.classic.column import _to_seq, _to_java_column
sc = _get_active_spark_context()
- expressions = cast(JVMView, sc._jvm).org.apache.spark.sql.catalyst.expressions
- expr = getattr(expressions, name)
-
- jcols = [_to_java_column(col).expr() for col in cols]
jfuns = [_create_lambda(f) for f in funs]
-
- return Column(cast(JVMView, sc._jvm).Column(expr(*jcols + jfuns)))
-
+ jcols = [_to_java_column(c) for c in cols]
+ return Column(sc._jvm.Column.pysparkFn(name, _to_seq(sc, jcols + jfuns)))
Review Comment:
Ah, the `Column.fn` has to be decorated with `scala.annotation.varargs` (which will create the same signature for variant arguments for Java).
--
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-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #47688: [SPARK-49022] Use Column Node API in Column
URL: https://github.com/apache/spark/pull/47688
--
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-49022] Use Column Node API in Column [spark]
Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2295226118
@hvanhovell what is the recommended way to migrate user code like
new Column(MyExpression())
to this Column Node Api?
The following are all `private[sql]` / `private[spark]`:
Column(MyExpression())
new Column(ExpressionColumnNode(MyExpression()))
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1714534304
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18669,18 +18665,12 @@ def _invoke_higher_order_function(
:return: a Column
"""
- from py4j.java_gateway import JVMView
- from pyspark.sql.classic.column import _to_java_column
+ from pyspark.sql.classic.column import _to_seq, _to_java_column
sc = _get_active_spark_context()
- expressions = cast(JVMView, sc._jvm).org.apache.spark.sql.catalyst.expressions
- expr = getattr(expressions, name)
-
- jcols = [_to_java_column(col).expr() for col in cols]
jfuns = [_create_lambda(f) for f in funs]
-
- return Column(cast(JVMView, sc._jvm).Column(expr(*jcols + jfuns)))
-
+ jcols = [_to_java_column(c) for c in cols]
+ return Column(sc._jvm.Column.pysparkFn(name, _to_seq(sc, jcols + jfuns)))
Review Comment:
@HyukjinKwon I cannot invoke `Column.fn(...)` here. Do you know why? I added `Column.pysparkFn(...)` as a workaround.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719249740
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18787,7 +18771,7 @@ def exists(col: "ColumnOrName", f: Callable[[Column], Column]) -> Column:
| true|
+------------+
"""
- return _invoke_higher_order_function("ArrayExists", [col], [f])
+ return _invoke_higher_order_function("exists", [col], [f])
Review Comment:
Probably I am overthinking but just for the sake of review, this will now call whatever `exists` is registered as, e.g., users could replace the existing function via `spark.register`.
--
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-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2294502869
Merged 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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720328712
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
- def unapply(col: Column): Option[Expression] = Some(col.expr)
+ def apply(node: => ColumnNode): Column = withOrigin(new Column(node))
Review Comment:
This is what get:
```
org.apache.spark.SparkArithmeticException: [DIVIDE_BY_ZERO] Division by zero. Use `try_divide` to tolerate divisor being 0 and return NULL instead. If necessary set "spark.sql.ansi.enabled" to "false" to bypass this error. SQLSTATE: 22012
== DataFrame ==
"div" was called from
<init>(<console>:1)
```
So that checks out. TBH this part is almost the same as before.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720322576
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
@@ -183,7 +185,7 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
println(s"""
|/**
| * Register a deterministic Java UDF$i instance as user-defined function (UDF).
- | * @since $version
+ | * @since 1.3.0
Review Comment:
Fixed.
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -283,14 +279,18 @@ class Column(val expr: Expression) extends Logging {
* @since 1.3.0
*/
def ===(other: Any): Column = {
- val right = lit(other).expr
- if (this.expr == right) {
+ val right = lit(other)
+ checkTrivialPredicate(right)
+ fn("=", other)
+ }
+
+ private def checkTrivialPredicate(right: Column): Unit = {
+ if (this == right) {
logWarning(
log"Constructing trivially true equals predicate, " +
- log"'${MDC(LEFT_EXPR, this.expr)} = ${MDC(RIGHT_EXPR, right)}'. " +
+ log"'${MDC(LEFT_EXPR, this)} <=> ${MDC(RIGHT_EXPR, right)}'. " +
Review Comment:
Done
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719257293
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1590,6 +1590,8 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
} else {
a.copy(aggregateExpressions = buildExpandedProjectList(a.aggregateExpressions, a.child))
}
+ case c: CollectMetrics if containsStar(c.metrics) =>
+ c.copy(metrics = buildExpandedProjectList(c.metrics, c.child))
Review Comment:
This part, I couldn't follow. Would you mind briefly explaining this please?
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719251585
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18787,7 +18771,7 @@ def exists(col: "ColumnOrName", f: Callable[[Column], Column]) -> Column:
| true|
+------------+
"""
- return _invoke_higher_order_function("ArrayExists", [col], [f])
+ return _invoke_higher_order_function("exists", [col], [f])
Review Comment:
...and all functions used on the scala side...
We can put it in a different namespace (e.g. builtin) - much like the internal thing we recently added - but that is for a different PR.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720310675
##########
sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -212,3 +183,25 @@ private[sql] case class UserDefinedAggregator[IN, BUF, OUT](
override def name: String = givenName.getOrElse(aggregator.name)
}
+
+private[sql] object UserDefinedFunctionUtils {
Review Comment:
No, UserDefinedFunction will be implementation agnostic. This should be moved to internal as well.
--
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-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2294497083
Tests have passed. I am merging this 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-49022] Use Column Node API in Column [spark]
Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2298881723
Currently working around this by gaining access to `Column(Expression)` in `private[spark]` package: https://github.com/G-Research/spark-extension/pull/256/files
--
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-49022] Use Column Node API in Column [spark]
Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on PR #47688:
URL: https://github.com/apache/spark/pull/47688#issuecomment-2295348116
> ... in that case we can still open up an expression way of creating a Column.
That would shortcut a lot of extra complexity and simplify backward compatibility.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719260846
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
@@ -183,7 +185,7 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
println(s"""
|/**
| * Register a deterministic Java UDF$i instance as user-defined function (UDF).
- | * @since $version
+ | * @since 1.3.0
Review Comment:
typo
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
@@ -483,15 +485,15 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
val udaf = clazz.getConstructor().newInstance().asInstanceOf[UserDefinedAggregateFunction]
register(name, udaf)
} catch {
- case e: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
- case e @ (_: InstantiationException | _: IllegalArgumentException) =>
+ case _: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
+ case _: InstantiationException | _: IllegalArgumentException =>
throw QueryCompilationErrors.classWithoutPublicNonArgumentConstructorError(className)
}
}
/**
* Register a deterministic Java UDF0 instance as user-defined function (UDF).
- * @since 2.3.0
+ * @since 1.3.0
Review Comment:
typo
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1719259122
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
Review Comment:
I will give it a better name.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720322367
##########
sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -212,3 +183,25 @@ private[sql] case class UserDefinedAggregator[IN, BUF, OUT](
override def name: String = givenName.getOrElse(aggregator.name)
}
+
+private[sql] object UserDefinedFunctionUtils {
Review Comment:
Moved it.
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
@@ -483,15 +485,15 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
val udaf = clazz.getConstructor().newInstance().asInstanceOf[UserDefinedAggregateFunction]
register(name, udaf)
} catch {
- case e: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
- case e @ (_: InstantiationException | _: IllegalArgumentException) =>
+ case _: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
+ case _: InstantiationException | _: IllegalArgumentException =>
throw QueryCompilationErrors.classWithoutPublicNonArgumentConstructorError(className)
}
}
/**
* Register a deterministic Java UDF0 instance as user-defined function (UDF).
- * @since 2.3.0
+ * @since 1.3.0
Review Comment:
Fixed.
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720322852
##########
sql/core/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
-import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils}
+import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression
-import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
import org.apache.spark.sql.expressions.Window
import org.apache.spark.sql.functions.lit
-import org.apache.spark.sql.internal.TypedAggUtils
+import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper}
import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
-private[sql] object Column {
+private[spark] object Column {
def apply(colName: String): Column = new Column(colName)
- def apply(expr: Expression): Column = new Column(expr)
+ def apply(expr: Expression): Column = Column(Wrapper(expr))
Review Comment:
Renamed to ExpressionColumnNode
--
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] [WIP][SPARK-49022] Use Column Node API in Column [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47688:
URL: https://github.com/apache/spark/pull/47688#discussion_r1720311426
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
@@ -483,15 +485,15 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
val udaf = clazz.getConstructor().newInstance().asInstanceOf[UserDefinedAggregateFunction]
register(name, udaf)
} catch {
- case e: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
- case e @ (_: InstantiationException | _: IllegalArgumentException) =>
+ case _: ClassNotFoundException => throw QueryCompilationErrors.cannotLoadClassNotOnClassPathError(className)
+ case _: InstantiationException | _: IllegalArgumentException =>
throw QueryCompilationErrors.classWithoutPublicNonArgumentConstructorError(className)
}
}
/**
* Register a deterministic Java UDF0 instance as user-defined function (UDF).
- * @since 2.3.0
+ * @since 1.3.0
Review Comment:
Funny
--
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