cloud-fan commented on code in PR #55682:
URL: https://github.com/apache/spark/pull/55682#discussion_r3212776701
##########
sql/connect/common/src/main/scala/org/apache/spark/sql/connect/Dataset.scala:
##########
@@ -421,6 +422,51 @@ class Dataset[T] private[sql] (
lateralJoin(right, Some(joinExprs), joinType)
}
+ private def nearestByJoinImpl(
+ right: sql.Dataset[_],
+ rankingExpression: Column,
+ numResults: Int,
+ joinType: String,
+ mode: String,
+ direction: String): DataFrame = {
+ // Validate locally so Connect users see the same errors as the classic
path without a
+ // server round-trip. Acceptance lists must stay aligned with
`NearestByJoinType` /
+ // `NearestByJoinMode` / `NearestByDirection` in sql/catalyst, which
`sql/connect/common`
+ // cannot import.
Review Comment:
After extracting `NearestByJoinValidation`, both sides reference the same
source — there are no separate lists left to "stay aligned" with. The bit worth
keeping is that the validation **logic** here mirrors what
`NearestByJoinType.apply` etc. do server-side, since `sql/connect/common` can't
import sql/catalyst.
```suggestion
// server round-trip. The validation logic mirrors
`NearestByJoinType.apply` /
// `NearestByJoinMode.apply` / `NearestByDirection.apply` in
sql/catalyst, which
// `sql/connect/common` cannot import; the acceptance lists themselves
are shared via
// `NearestByJoinValidation` in sql-api.
```
##########
sql/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -1276,3 +1277,33 @@ message LateralJoin {
// (Required) The join type.
Join.JoinType join_type = 4;
}
+
+// Relation of type [[NearestByJoin]].
+//
+// For each row on the left side, returns up to `num_results` rows from the
right side ordered
+// by `ranking_expression`.
Review Comment:
Top-K, not a sort — output rows aren't guaranteed to be ordered. The
Scaladoc and Python docstring already say "ranked by".
```suggestion
// For each row on the left side, returns up to `num_results` rows from the
right side ranked
// by `ranking_expression`.
```
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameNearestByJoinSuite.scala:
##########
@@ -0,0 +1,442 @@
+/*
+ * 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
+
+import org.apache.spark.sql.catalyst.plans.{NearestByDirection,
NearestByJoinMode, NearestByJoinType}
+import org.apache.spark.sql.execution.streaming.runtime.MemoryStream
+import org.apache.spark.sql.functions._
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.tags.SlowSQLTest
+
+@SlowSQLTest
+class DataFrameNearestByJoinSuite extends QueryTest with SharedSparkSession {
+
+ private def prepareForNearestByJoin(): (classic.DataFrame,
classic.DataFrame) = {
+ val users = spark.createDataFrame(
+ Seq((1, 10.0), (2, 20.0), (3, 30.0))).toDF("user_id", "score")
+ val products = spark.createDataFrame(
+ Seq(("A", 11.0), ("B", 22.0), ("C", 5.0))).toDF("product", "pscore")
+ (users, products)
+ }
+
+ test("similarity, inner, k=1") {
+ val (users, products) = prepareForNearestByJoin()
+ val result = users.nearestByJoin(
+ products,
+ -abs(users("score") - products("pscore")),
+ numResults = 1,
+ mode = "exact",
+ direction = "similarity")
+
+ checkAnswer(
+ result.select("user_id", "product").orderBy("user_id"),
+ Seq(Row(1, "A"), Row(2, "B"), Row(3, "B"))
+ )
+ }
+
+ test("distance, inner, k=2") {
+ val (users, products) = prepareForNearestByJoin()
+ val result = users.nearestByJoin(
+ products,
+ abs(users("score") - products("pscore")),
+ numResults = 2,
+ mode = "exact",
+ direction = "distance")
+
+ // For each user_id, closest 2 by |score - pscore|:
+ // user 1 (10): A (|10-11|=1), C (|10-5|=5)
+ // user 2 (20): B (|20-22|=2), A (|20-11|=9)
+ // user 3 (30): B (|30-22|=8), A (|30-11|=19)
+ checkAnswer(
+ result.select("user_id", "product").orderBy("user_id", "product"),
+ Seq(
+ Row(1, "A"), Row(1, "C"),
+ Row(2, "A"), Row(2, "B"),
+ Row(3, "A"), Row(3, "B"))
+ )
+ }
+
+ test("left outer when right side is empty") {
+ val (users, products) = prepareForNearestByJoin()
+ val emptyProducts = products.filter(lit(false))
+ val result = users.nearestByJoin(
+ emptyProducts,
+ -abs(users("score") - emptyProducts("pscore")),
+ numResults = 1,
+ joinType = "leftouter",
+ mode = "approx",
+ direction = "similarity")
+
+ checkAnswer(
+ result.select("user_id", "product").orderBy("user_id"),
+ Seq(Row(1, null), Row(2, null), Row(3, null))
+ )
+ }
+
+ test("inner drops left rows with no matches") {
+ val (users, products) = prepareForNearestByJoin()
+ val emptyProducts = products.filter(lit(false))
+ val result = users.nearestByJoin(
+ emptyProducts,
+ -abs(users("score") - emptyProducts("pscore")),
+ numResults = 1,
+ mode = "exact",
+ direction = "similarity")
+
+ assert(result.count() === 0)
+ }
+
+ test("self-join: each row finds nearest other rows in the same DataFrame") {
+ val (users, _) = prepareForNearestByJoin()
+ // For each user, find the 1 other user with the closest score (excluding
self by ranking).
Review Comment:
The comment says "find the 1 other user with the closest score (excluding
self by ranking)", but the ranking expression `-abs(users("score") -
users("score"))` resolves to `left.score - left.score` (both column references
were captured from the same Dataset before `DeduplicateRelations` re-IDs the
right side), so the rank is identically 0 for every candidate — top-K just
returns any 2 of the 3 right rows; there is no self-exclusion. The test still
does what it needs to (verifies the self-join resolves at all), but the comment
overstates what's verified.
```suggestion
// We pass `users` as both sides; DeduplicateRelations rewrites the
right side to
// generate fresh ExprIds, so the join resolves. Both `users("score")`
references in
// the ranking expression bind to the original (left) attribute, so the
rank is
// identically 0 for every candidate -- this test exercises self-join
resolution,
// not nearest-row selection.
```
##########
python/pyspark/sql/dataframe.py:
##########
@@ -2870,6 +2870,73 @@ def lateralJoin(
"""
...
+ def nearestByJoin(
+ self,
+ other: "DataFrame",
+ rankingExpression: Column,
+ numResults: int,
+ mode: str,
+ direction: str,
+ *,
+ joinType: str = "inner",
+ ) -> "DataFrame":
+ """
+ Nearest-by top-K ranking join with another :class:`DataFrame`. For
each row on the
+ left (query side), returns up to ``numResults`` rows from ``other``
(base side), ranked
+ by ``rankingExpression``.
+
+ The current implementation evaluates the full cross-product of left
and right and
+ bounds memory per left row by ``numResults``. Index-backed approximate
strategies
+ (transparent to ``approx`` mode) are planned for a future release;
until then,
+ pre-filter ``other`` when it is large. Tie-breaking among rows with
equal ranking
+ values is unspecified.
+
+ .. versionadded:: 4.2.0
+
+ Parameters
+ ----------
+ other : :class:`DataFrame`
+ Right (base side) of the join - the candidate pool searched for
each row of this
+ DataFrame.
+ rankingExpression : :class:`Column`
+ Scalar expression used to rank candidate rows on the right side.
+ numResults : int
+ Maximum number of matches per query row. Must be between 1 and
100000.
+ mode : str
+ Search algorithm contract. Must be one of: ``approx``, ``exact``.
``approx`` allows
+ the optimizer to use indexed or other approximate strategies when
available;
+ ``exact`` forces brute-force evaluation and requires the ranking
expression to be
+ deterministic.
+ direction : str
+ ``"distance"`` (smallest values first) or ``"similarity"``
(largest values first).
Review Comment:
Python uses the plural form here, but the Scala 5-arg and 6-arg overloads at
`sql/api/.../Dataset.scala:941,971` use singular ("smallest value first" /
"largest value first"). Either is fine; consistency matters more.
```suggestion
``"distance"`` (smallest value first) or ``"similarity"``
(largest value first).
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
