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/22 02:06:02 UTC
[PR] [SPARK-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
hvanhovell opened a new pull request, #47839:
URL: https://github.com/apache/spark/pull/47839
### What changes were proposed in this pull request?
This PR moves the sql Column to sql/api. A (foreseen) consequence of this is that we need to move the Spark Connect Scala Client to the shared API. This makes the PR quite large.
### Why are the changes needed?
We want to create a Scala Client interface that is shared between Classic and Connect.
### Does this PR introduce _any_ user-facing change?
Connect does no support untyped Scala UDFs until we figure out a solution for the conf system.
### How was this patch tested?
Mostly existing tests. I had to change a couple of tests on the connect side:
- UDAF related tests used a feature that was not supposed to work in the first place. You could use a typed aggregator on an untyped Dataset (Row). I have added a test for the scenario where you use an untyped aggregator on an untyped dataset.
- In some cases the shared Column API generates slightly different structures (more/less arguments, different names, or different structure). The verbatims used by `PlanGenerationTestSuite` and `ProtoToParsedPlanTestSuite` were updated to reflect this. The protos that had non trivial changes were renamed with the `_orphaned` suffix, this to make sure we keep testing plans produced by older clients.
### 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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726149232
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/connect/columnNodeSupport.scala:
##########
@@ -89,58 +103,65 @@ object ColumnNodeToProtoConverter extends (ColumnNode => proto.Expression) {
builder.getExpressionStringBuilder.setExpression(expression)
case s: SortOrder =>
- builder.setSortOrder(convertSortOrder(s))
+ builder.setSortOrder(convertSortOrder(s, e))
case Window(windowFunction, windowSpec, _) =>
val b = builder.getWindowBuilder
- .setWindowFunction(apply(windowFunction))
- .addAllPartitionSpec(windowSpec.partitionColumns.map(apply).asJava)
- .addAllOrderSpec(windowSpec.sortColumns.map(convertSortOrder).asJava)
+ .setWindowFunction(apply(windowFunction, e))
+ .addAllPartitionSpec(windowSpec.partitionColumns.map(apply(_, e)).asJava)
+ .addAllOrderSpec(windowSpec.sortColumns.map(convertSortOrder(_, e)).asJava)
windowSpec.frame.foreach { frame =>
b.getFrameSpecBuilder
.setFrameType(frame.frameType match {
case WindowFrame.Row => FrameType.FRAME_TYPE_ROW
case WindowFrame.Range => FrameType.FRAME_TYPE_RANGE
})
- .setLower(convertFrameBoundary(frame.lower))
- .setUpper(convertFrameBoundary(frame.upper))
+ .setLower(convertFrameBoundary(frame.lower, e))
+ .setUpper(convertFrameBoundary(frame.upper, e))
}
case UnresolvedExtractValue(child, extraction, _) =>
builder.getUnresolvedExtractValueBuilder
- .setChild(apply(child))
- .setExtraction(apply(extraction))
+ .setChild(apply(child, e))
+ .setExtraction(apply(extraction, e))
case UpdateFields(structExpression, fieldName, valueExpression, _) =>
val b = builder.getUpdateFieldsBuilder
- .setStructExpression(apply(structExpression))
+ .setStructExpression(apply(structExpression, e))
.setFieldName(fieldName)
- valueExpression.foreach(v => b.setValueExpression(apply(v)))
+ valueExpression.foreach(v => b.setValueExpression(apply(v, e)))
case v: UnresolvedNamedLambdaVariable =>
builder.setUnresolvedNamedLambdaVariable(convertNamedLambdaVariable(v))
case LambdaFunction(function, arguments, _) =>
builder.getLambdaFunctionBuilder
- .setFunction(apply(function))
+ .setFunction(apply(function, e))
.addAllArguments(arguments.map(convertNamedLambdaVariable).asJava)
- case InvokeInlineUserDefinedFunction(udf: ScalaUserDefinedFunction, arguments, false, _) =>
- val b = builder.getCommonInlineUserDefinedFunctionBuilder
- .setScalarScalaUdf(udf.udf)
- .setDeterministic(udf.deterministic)
- .addAllArguments(arguments.map(apply).asJava)
- udf.givenName.foreach(b.setFunctionName)
+ case InvokeInlineUserDefinedFunction(
+ a: Aggregator[Any @unchecked, Any @unchecked, Any @unchecked],
+ Nil,
+ false,
+ _) =>
+ // TODO we should probably 'just' detect this particular scenario
+ // in the planner instead of wrapping it in a separate method.
+ val protoUdf = UdfToProtoUtils.toProto(UserDefinedAggregator(a, e.get))
+ builder.getTypedAggregateExpressionBuilder.setScalarScalaUdf(protoUdf.getScalarScalaUdf)
+
+ case InvokeInlineUserDefinedFunction(udf: UserDefinedFunction, args, false, _) =>
+ builder.setCommonInlineUserDefinedFunction(
+ UdfToProtoUtils.toProto(udf, args.map(apply(_, e))))
Review Comment:
Please note that we are serializing the UDF here. This is much later than before (when it is being used in the plan, instead of when it is being defined). The reason for doing it earlier was to mostly avoid capturing unwanted classes in the REPL. We had test for this, and those are passing. That means that the closure cleaner work had paid off.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726152974
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/trees/origin.scala:
##########
@@ -84,6 +87,49 @@ object CurrentOrigin {
val ret = try f finally { set(previous) }
ret
}
+
+ /**
+ * This helper function captures the Spark API and its call site in the user code from the current
+ * stacktrace.
+ *
+ * As adding `withOrigin` explicitly to all Spark API definition would be a huge change,
+ * `withOrigin` is used only at certain places where all API implementation surely pass through
+ * and the current stacktrace is filtered to the point where first Spark API code is invoked from
+ * the user code.
+ *
+ * As there might be multiple nested `withOrigin` calls (e.g. any Spark API implementations can
+ * invoke other APIs) only the first `withOrigin` is captured because that is closer to the user
+ * code.
+ *
+ * @param f The function that can use the origin.
+ * @return The result of `f`.
+ */
+ private[sql] def withOrigin[T](f: => T): T = {
Review Comment:
Moved here so we could use it in sql/api.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726147190
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
Review Comment:
This is shared now.
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/expressions/Window.scala:
##########
Review Comment:
This is shared now.
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/expressions/WindowSpec.scala:
##########
Review Comment:
This is shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726145976
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/MergeIntoWriter.scala:
##########
@@ -234,6 +234,26 @@ class MergeIntoWriter[T] private[sql] (
this.notMatchedBySourceActions = this.notMatchedBySourceActions :++ action
this
}
+
+ private[sql] def buildMergeAction(
Review Comment:
This was taken from the 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] [SPARK-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726147089
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/connect/columnNodeSupport.scala:
##########
@@ -20,20 +20,30 @@ import scala.jdk.CollectionConverters._
import org.apache.spark.SparkException
import org.apache.spark.connect.proto
+import org.apache.spark.connect.proto.Expression
import org.apache.spark.connect.proto.Expression.SortOrder.NullOrdering.{SORT_NULLS_FIRST, SORT_NULLS_LAST}
import org.apache.spark.connect.proto.Expression.SortOrder.SortDirection.{SORT_DIRECTION_ASCENDING, SORT_DIRECTION_DESCENDING}
import org.apache.spark.connect.proto.Expression.Window.WindowFrame.{FrameBoundary, FrameType}
+import org.apache.spark.sql.{Column, Encoder}
import org.apache.spark.sql.catalyst.trees.{CurrentOrigin, Origin}
import org.apache.spark.sql.connect.common.DataTypeProtoConverter
import org.apache.spark.sql.connect.common.LiteralValueProtoConverter.toLiteralProtoBuilder
-import org.apache.spark.sql.expressions.ScalaUserDefinedFunction
+import org.apache.spark.sql.expressions.{Aggregator, UserDefinedAggregator, UserDefinedFunction}
import org.apache.spark.sql.internal._
/**
* Converter for [[ColumnNode]] to [[proto.Expression]] conversions.
*/
object ColumnNodeToProtoConverter extends (ColumnNode => proto.Expression) {
- override def apply(node: ColumnNode): proto.Expression = {
+ def toExpr(column: Column): proto.Expression = apply(column.node, None)
+
+ def toTypedExpr[I](column: Column, encoder: Encoder[I]): proto.Expression = {
+ apply(column.node, Option(encoder))
+ }
+
+ override def apply(node: ColumnNode): Expression = apply(node, None)
+
+ private def apply(node: ColumnNode, e: Option[Encoder[_]]): proto.Expression = {
Review Comment:
We need the encoder for Aggregator's without an input encoder.
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/expressions/Aggregator.scala:
##########
Review Comment:
This is shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726147324
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
Review Comment:
This is shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726153218
##########
sql/api/src/main/scala/org/apache/spark/sql/errors/CompilationErrors.scala:
##########
@@ -49,6 +49,34 @@ private[sql] trait CompilationErrors extends DataTypeErrorsBase {
messageParameters = Map("filePath" -> filePath),
cause = Option(cause))
}
+
+ def usingUntypedScalaUDFError(): Throwable = {
Review Comment:
Moved here because it is used in WindowSpec.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #47839:
URL: https://github.com/apache/spark/pull/47839#issuecomment-2307573229
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726157794
##########
sql/core/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
Review Comment:
Mostly shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726147791
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/internal/UdfToProtoUtils.scala:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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 org.apache.spark.sql.internal
+
+import scala.collection.mutable
+import scala.jdk.CollectionConverters._
+import scala.util.control.NonFatal
+
+import com.google.protobuf.ByteString
+
+import org.apache.spark.SparkException
+import org.apache.spark.connect.proto
+import org.apache.spark.sql.catalyst.encoders.{AgnosticEncoder, RowEncoder}
+import org.apache.spark.sql.connect.common.DataTypeProtoConverter.toConnectProtoType
+import org.apache.spark.sql.connect.common.UdfPacket
+import org.apache.spark.sql.encoderFor
+import org.apache.spark.sql.expressions.{SparkUserDefinedFunction, UserDefinedAggregator, UserDefinedFunction}
+import org.apache.spark.util.{ClosureCleaner, SparkClassUtils, SparkSerDeUtils}
+
+/**
+ * Utility for converting a [[UserDefinedFunction]] into a Connect Protobuf message.
+ */
+private[sql] object UdfToProtoUtils {
Review Comment:
This contains most of the code that used to be part of the `ScalaUserDefinedFunction` companion.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726153938
##########
sql/api/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3651,7 +3652,7 @@ object functions {
* @group math_funcs
* @since 3.5.0
*/
- def random(seed: Column): Column = call_function("random", seed)
+ def random(seed: Column): Column = Column.fn("random", seed)
Review Comment:
This one was a bit unexpected :)...
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726152614
##########
sql/api/src/main/scala/org/apache/spark/sql/api/Dataset.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * 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 org.apache.spark.sql.api
+
+abstract class Dataset[E] {
Review Comment:
We currently only need it to make broadcast work.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726152354
##########
sql/api/src/main/scala/org/apache/spark/sql/api/Dataset.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * 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 org.apache.spark.sql.api
+
+abstract class Dataset[E] {
Review Comment:
This will be (vastly) expanded in SPARK-49029.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726150344
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/UserDefinedFunctionE2ETestSuite.scala:
##########
@@ -315,13 +315,20 @@ class UserDefinedFunctionE2ETestSuite extends QueryTest with RemoteSparkSession
"c")
}
- test("(deprecated) scala UDF with dataType") {
+ // TODO re-enable this after we hooked SqlApiConf into the session confs.
Review Comment:
We need a configurable SqlApiConf for the client to make this work.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726150923
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/UserDefinedFunctionE2ETestSuite.scala:
##########
@@ -380,23 +387,23 @@ class UserDefinedFunctionE2ETestSuite extends QueryTest with RemoteSparkSession
}
test("UDAF custom Aggregator - toColumn") {
- val session: SparkSession = spark
- import session.implicits._
+ val encoder = Encoders.product[UdafTestInput]
val aggCol = new CompleteUdafTestInputAggregator().toColumn
- val ds = spark.range(10).withColumn("extra", col("id") * 2).as[UdafTestInput]
-
+ val ds = spark.range(10).withColumn("extra", col("id") * 2).as(encoder)
assert(ds.select(aggCol).head() == 135) // 45 + 90
- assert(ds.agg(aggCol).head().getLong(0) == 135) // 45 + 90
Review Comment:
This does not work in classic either :)... So removing it. I have added a test case below to high light what is supposed to work. cc @xupefei
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726151277
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/ColumnNodeToProtoConverterSuite.scala:
##########
Review Comment:
Additional testing for the new UDF types.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726161416
##########
sql/api/src/main/scala/org/apache/spark/sql/expressions/Aggregator.scala:
##########
Review Comment:
Shared now!
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726159397
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/PlanGenerationTestSuite.scala:
##########
Review Comment:
Added a couple of test cases here because they are producing different proto plans than before. The shares API tends to send fewer columns if it can, and it uses sql schemas instead of json schemas for conversions.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726145302
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/Column.scala:
##########
Review Comment:
This is shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726147089
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/connect/columnNodeSupport.scala:
##########
@@ -20,20 +20,30 @@ import scala.jdk.CollectionConverters._
import org.apache.spark.SparkException
import org.apache.spark.connect.proto
+import org.apache.spark.connect.proto.Expression
import org.apache.spark.connect.proto.Expression.SortOrder.NullOrdering.{SORT_NULLS_FIRST, SORT_NULLS_LAST}
import org.apache.spark.connect.proto.Expression.SortOrder.SortDirection.{SORT_DIRECTION_ASCENDING, SORT_DIRECTION_DESCENDING}
import org.apache.spark.connect.proto.Expression.Window.WindowFrame.{FrameBoundary, FrameType}
+import org.apache.spark.sql.{Column, Encoder}
import org.apache.spark.sql.catalyst.trees.{CurrentOrigin, Origin}
import org.apache.spark.sql.connect.common.DataTypeProtoConverter
import org.apache.spark.sql.connect.common.LiteralValueProtoConverter.toLiteralProtoBuilder
-import org.apache.spark.sql.expressions.ScalaUserDefinedFunction
+import org.apache.spark.sql.expressions.{Aggregator, UserDefinedAggregator, UserDefinedFunction}
import org.apache.spark.sql.internal._
/**
* Converter for [[ColumnNode]] to [[proto.Expression]] conversions.
*/
object ColumnNodeToProtoConverter extends (ColumnNode => proto.Expression) {
- override def apply(node: ColumnNode): proto.Expression = {
+ def toExpr(column: Column): proto.Expression = apply(column.node, None)
+
+ def toTypedExpr[I](column: Column, encoder: Encoder[I]): proto.Expression = {
+ apply(column.node, Option(encoder))
+ }
+
+ override def apply(node: ColumnNode): Expression = apply(node, None)
+
+ private def apply(node: ColumnNode, e: Option[Encoder[_]]): proto.Expression = {
Review Comment:
We need the encoder for Aggregator's without an input encoder. This is similar to the withInputTypes calls used in classic.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726158059
##########
sql/core/src/main/scala/org/apache/spark/sql/package.scala:
##########
@@ -77,47 +73,4 @@ package object sql {
* with rebasing.
*/
private[sql] val SPARK_LEGACY_INT96_METADATA_KEY = "org.apache.spark.legacyINT96"
-
- /**
- * This helper function captures the Spark API and its call site in the user code from the current
- * stacktrace.
- *
- * As adding `withOrigin` explicitly to all Spark API definition would be a huge change,
- * `withOrigin` is used only at certain places where all API implementation surely pass through
- * and the current stacktrace is filtered to the point where first Spark API code is invoked from
- * the user code.
- *
- * As there might be multiple nested `withOrigin` calls (e.g. any Spark API implementations can
- * invoke other APIs) only the first `withOrigin` is captured because that is closer to the user
- * code.
- *
- * @param f The function that can use the origin.
- * @return The result of `f`.
- */
- private[sql] def withOrigin[T](f: => T): T = {
Review Comment:
Moved to CurrentOrigin.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726160904
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/FunctionTestSuite.scala:
##########
Review Comment:
Because of the switch to the shared API these produce different (proto) expression trees now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #47839: [SPARK-49027][CONNECT][SQL] Share Column API between Class and Connect
URL: https://github.com/apache/spark/pull/47839
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726154637
##########
sql/api/src/main/scala/org/apache/spark/sql/internal/ToScalaUDF.scala:
##########
@@ -22,7 +22,8 @@ import org.apache.spark.sql.api.java._
* Helper class that provides conversions from org.apache.spark.sql.api.java.Function* to
* scala.Function*.
*/
-private[sql] object ToScalaUDF {
+@SerialVersionUID(2019907615267866045L)
Review Comment:
This is used in connect now. We are fixing the UID to avoid trouble down the line.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726153495
##########
sql/api/src/main/scala/org/apache/spark/sql/expressions/Window.scala:
##########
Review Comment:
Shared now!
##########
sql/api/src/main/scala/org/apache/spark/sql/expressions/WindowSpec.scala:
##########
Review Comment:
Shared now!
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726149785
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ColumnTestSuite.scala:
##########
@@ -170,11 +170,11 @@ class ColumnTestSuite extends ConnectFunSuite {
val x = fn.col("a") + fn.col("b")
val explain1 = captureStdOut(x.explain(false))
val explain2 = captureStdOut(x.explain(true))
- assert(explain1 == explain2)
- val expectedFragments = Seq("unresolved_function", "function_name: \"+\"", "arguments")
- expectedFragments.foreach { fragment =>
- assert(explain1.contains(fragment))
- }
+ assert(explain1 != explain2)
+ assert(explain1.strip() == "+(a, b)")
Review Comment:
The textual representation of the plan is different now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726146361
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/UDFRegistration.scala:
##########
Review Comment:
Most of this is shared now.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726187085
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##########
@@ -300,17 +297,35 @@ object CheckConnectJvmClientCompatibility {
ProblemFilters.exclude[DirectMissingMethodProblem](
"org.apache.spark.sql.UDFRegistration.register"),
- // Typed Column
- ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.sql.TypedColumn.*"),
- ProblemFilters.exclude[IncompatibleResultTypeProblem](
- "org.apache.spark.sql.TypedColumn.expr"),
- ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.TypedColumn$"),
-
// ColumnNode conversions
ProblemFilters.exclude[DirectMissingMethodProblem](
"org.apache.spark.sql.SparkSession.Converter"),
ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.SparkSession$Converter$"),
- ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.SparkSession$RichColumn"),
+
+ // Missing expressions.
+ ProblemFilters.exclude[MissingClassProblem](
+ "org.apache.spark.sql.expressions.ReduceAggregator"),
+ ProblemFilters.exclude[MissingClassProblem](
+ "org.apache.spark.sql.expressions.ReduceAggregator$"),
+ ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.expressions.javalang.*"),
+ ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.expressions.scalalang.*"),
+
+ // UDFRegistration
Review Comment:
This weird thing is that the MiMa check misses the one missing method we do care about :/...
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726151651
##########
sql/api/src/main/scala/org/apache/spark/sql/Column.scala:
##########
@@ -35,6 +36,25 @@ private[spark] object Column {
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
+ * column is dropped.
+ */
+ private[sql] def fnWithOptions(
Review Comment:
It made more sense to have it here than in functions.scala.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726153339
##########
sql/api/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:
##########
@@ -139,6 +140,18 @@ object SparkUserDefinedFunction {
nullable = outputEncoder.nullable)
}
+ private[sql] def apply(
Review Comment:
Constructor used by the Scala Client.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726152764
##########
sql/api/src/main/scala/org/apache/spark/sql/api/UDFRegistration.scala:
##########
Review Comment:
Shared UDFRegistration interface.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726154110
##########
sql/api/src/main/scala/org/apache/spark/sql/internal/SqlApiConf.scala:
##########
@@ -44,6 +44,8 @@ private[sql] trait SqlApiConf {
def sessionLocalTimeZone: String
def legacyTimeParserPolicy: LegacyBehaviorPolicy.Value
def defaultStringType: StringType
+ def stackTracesInDataFrameContext: Int
+ def legacyAllowUntypedScalaUDFs: Boolean
Review Comment:
Needed in functions.scala
##########
sql/api/src/main/scala/org/apache/spark/sql/internal/SqlApiConf.scala:
##########
@@ -44,6 +44,8 @@ private[sql] trait SqlApiConf {
def sessionLocalTimeZone: String
def legacyTimeParserPolicy: LegacyBehaviorPolicy.Value
def defaultStringType: StringType
+ def stackTracesInDataFrameContext: Int
Review Comment:
Needed in withOrigin
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726156433
##########
sql/connect/common/src/test/resources/query-tests/queries/function_from_json_orphaned.json:
##########
Review Comment:
Maybe we can without the orphaned one here because we explicitly added test for from_json with a json schema.
--
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-49027][CONNECT][SQL] Share Column API between Class and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #47839:
URL: https://github.com/apache/spark/pull/47839#discussion_r1726187085
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##########
@@ -300,17 +297,35 @@ object CheckConnectJvmClientCompatibility {
ProblemFilters.exclude[DirectMissingMethodProblem](
"org.apache.spark.sql.UDFRegistration.register"),
- // Typed Column
- ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.sql.TypedColumn.*"),
- ProblemFilters.exclude[IncompatibleResultTypeProblem](
- "org.apache.spark.sql.TypedColumn.expr"),
- ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.TypedColumn$"),
-
// ColumnNode conversions
ProblemFilters.exclude[DirectMissingMethodProblem](
"org.apache.spark.sql.SparkSession.Converter"),
ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.SparkSession$Converter$"),
- ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.SparkSession$RichColumn"),
+
+ // Missing expressions.
+ ProblemFilters.exclude[MissingClassProblem](
+ "org.apache.spark.sql.expressions.ReduceAggregator"),
+ ProblemFilters.exclude[MissingClassProblem](
+ "org.apache.spark.sql.expressions.ReduceAggregator$"),
+ ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.expressions.javalang.*"),
+ ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.expressions.scalalang.*"),
+
+ // UDFRegistration
Review Comment:
This weird thing is that the MiMa check the one missing method we do care about :/...
--
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