wangyum commented on code in PR #59027:
URL: https://github.com/apache/spark/pull/59027#discussion_r4256960183


##########
sql/core/src/test/scala/org/apache/spark/sql/RangeJoinSQLSuite.scala:
##########
@@ -0,0 +1,560 @@
+/*
+ * 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.SparkException
+import org.apache.spark.sql.catalyst.optimizer.{BuildLeft, BuildRight}
+import org.apache.spark.sql.catalyst.planning.{IntervalOverlapJoin, 
LessPartialRangeJoin}
+import org.apache.spark.sql.catalyst.plans.{Inner, LeftAnti, LeftOuter, 
LeftSemi}
+import org.apache.spark.sql.execution.{InputAdapter, ReusedSubqueryExec, 
ScalarSubquery, SparkPlan, SubqueryExec, WholeStageCodegenExec}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.execution.joins.{BroadcastNestedLoopJoinExec, 
BroadcastRangeJoinExec, CartesianProductExec}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.SharedSparkSession
+
+/**
+ * Planner and end-to-end coverage for broadcast range join.
+ *
+ * Which condition is recognized: `ExtractRangeJoinKeysSuite`.
+ * What the index returns: `RangeIndexSuite`.
+ * What the operator returns: `RangeJoinSuite`.
+ * This suite checks that the planner chooses the operator, and that the SQL
+ * result matches broadcast nested loop join where that comparison is the 
point.
+ */
+class RangeJoinSQLSuite extends QueryTest with SharedSparkSession with 
AdaptiveSparkPlanHelper {
+
+  private val pointCond =
+    "s.ip_add_int >= l.BEGIN_IP2LONG AND s.ip_add_int <= l.END_IP2LONG"
+
+  private def findRangeJoin(plan: SparkPlan): Seq[BroadcastRangeJoinExec] = {
+    collect(plan) { case j: BroadcastRangeJoinExec => j }
+  }
+
+  /** `n` range joins sit in one whole-stage, not in nested stages or input 
adapters. */
+  private def oneCodegenStageOwns(plan: SparkPlan, n: Int): Boolean = {
+    exists(plan) {
+      case w: WholeStageCodegenExec =>
+        def count(p: SparkPlan): Int = p match {
+          case _: InputAdapter | _: WholeStageCodegenExec => 0
+          case j: BroadcastRangeJoinExec => 1 + j.children.map(count).sum
+          case other => other.children.map(count).sum
+        }
+        count(w.child) == n
+      case _ => false
+    }
+  }
+
+  /** The range join is compiled into a whole-stage, not hidden under an input 
adapter. */
+  private def rangeJoinIsCodegen(plan: SparkPlan): Boolean = {
+    exists(plan) {
+      case w: WholeStageCodegenExec =>
+        def contains(p: SparkPlan): Boolean = p match {
+          case _: InputAdapter => false
+          case _: BroadcastRangeJoinExec => true
+          case other => other.children.exists(contains)
+        }
+        contains(w.child)
+      case _ => false
+    }
+  }
+
+  private def findBroadcastNestedLoopJoin(plan: SparkPlan): 
Seq[BroadcastNestedLoopJoinExec] = {
+    collect(plan) { case j: BroadcastNestedLoopJoinExec => j }
+  }
+
+  private def findCartesianProduct(plan: SparkPlan): Seq[CartesianProductExec] 
= {
+    collect(plan) { case c: CartesianProductExec => c }
+  }
+
+  private def withRangeJoin(extra: (String, String)*)(f: => Unit): Unit = {
+    withSQLConf((Seq(SQLConf.BROADCAST_RANGE_JOIN_ENABLED.key -> "true") ++ 
extra): _*)(f)
+  }
+
+  private def setupIpLookupViews(): Unit = {
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW signin_ip(id, ip_add_int) AS VALUES
+        |  ('1', 1467630666L),
+        |  ('2', 1186929717L),
+        |  ('3', 3501544352L),
+        |  ('31', 3501544320L),
+        |  ('32', 3501544383L),
+        |  ('33', 3501544330L),
+        |  ('34', 3501544390L),
+        |  ('4', 1626243295L),
+        |  ('41', 1626243072L),
+        |  ('42', 1626243327L),
+        |  ('5', 1206355995L),
+        |  ('5', 1206355995L),
+        |  ('6', CAST(NULL AS BIGINT)),
+        |  ('7', 0L)
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW ip_lookup(
+        |    BEGIN_IP2LONG, END_IP2LONG, TWO_CHAR_CNTRY_CD, zip_id) AS VALUES
+        |  (1467630592L, 1467630847L, 'de', 2380520L),
+        |  (1186929664L, 1186930175L, 'us', 6723936L),
+        |  (1186929664L, 1186930175L, 'us', 6723936L),
+        |  (3501544320L, 3501544383L, 'ca', 12804631L),
+        |  (3501544330L, 3501544390L, 'ca', 12804631001L),
+        |  (1626243072L, 1626243327L, 'us', 8849753L),
+        |  (1206355968L, 1206356223L, 'us', 10138801L),
+        |  (CAST(NULL AS BIGINT), 1206356223L, 'us', 10138801L),
+        |  (1206355968L, CAST(NULL AS BIGINT), 'us', 10138801L),
+        |  (CAST(NULL AS BIGINT), CAST(NULL AS BIGINT), 'us', 10138801L)
+        |""".stripMargin)
+  }
+
+  private def setupIntervalViews(): Unit = {
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW intervals_a(lo, hi) AS VALUES
+        |  (-1, 0), (0, 1), (0, 2), (1, 5)
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW intervals_b(lo, hi) AS VALUES
+        |  (-2, -1), (-4, -2), (1, 3), (5, 7)
+        |""".stripMargin)
+  }
+
+  private def setupDecimalViews(): Unit = {
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW dec_ranges(lo, hi) AS VALUES
+        |  (CAST(0.00 AS DECIMAL(10, 2)), CAST(2.50 AS DECIMAL(10, 2))),
+        |  (CAST(3.00 AS DECIMAL(10, 2)), CAST(5.00 AS DECIMAL(10, 2)))
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW dec_points(p) AS VALUES
+        |  (CAST(1.25 AS DECIMAL(10, 2))),
+        |  (CAST(3.00 AS DECIMAL(10, 2))),
+        |  (CAST(9.00 AS DECIMAL(10, 2)))
+        |""".stripMargin)
+  }
+
+  private def setupCollatedStrings(): Unit = {
+    // 'M' and 'm' compare equal under UNICODE_CI and differ as bytes.
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW str_ranges(lo, hi) AS VALUES
+        |  (CAST('a' AS STRING COLLATE UNICODE_CI), CAST('a' AS STRING COLLATE 
UNICODE_CI)),
+        |  (CAST('b' AS STRING COLLATE UNICODE_CI), CAST('b' AS STRING COLLATE 
UNICODE_CI)),
+        |  (CAST('c' AS STRING COLLATE UNICODE_CI), CAST('M' AS STRING COLLATE 
UNICODE_CI)),
+        |  (CAST('m' AS STRING COLLATE UNICODE_CI), CAST('z' AS STRING COLLATE 
UNICODE_CI))
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW str_points(s) AS VALUES
+        |  (CAST('M' AS STRING COLLATE UNICODE_CI)),
+        |  (CAST('m' AS STRING COLLATE UNICODE_CI))
+        |""".stripMargin)
+  }
+
+  private def setupDateAndTimestampViews(): Unit = {
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW dt_ranges(lo, hi) AS VALUES
+        |  (DATE'2024-01-01', DATE'2024-01-31'),
+        |  (DATE'2024-06-01', DATE'2024-06-30')
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW dt_points(d) AS VALUES
+        |  (DATE'2024-01-15'),
+        |  (DATE'2024-05-01'),
+        |  (DATE'2024-06-15')
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW ts_ranges(lo, hi) AS VALUES
+        |  (TIMESTAMP'2024-01-01 00:00:00', TIMESTAMP'2024-01-01 12:00:00')
+        |""".stripMargin)
+    sql(
+      """
+        |CREATE OR REPLACE TEMP VIEW ts_points(t) AS VALUES
+        |  (TIMESTAMP'2024-01-01 06:00:00'),
+        |  (TIMESTAMP'2024-01-01 18:00:00')
+        |""".stripMargin)
+  }
+
+  test("range join is disabled by default and falls back to nested loop join") 
{
+    setupIpLookupViews()
+    val df = sql(s"SELECT s.id, l.zip_id FROM signin_ip s JOIN ip_lookup l ON 
$pointCond")
+    assert(findRangeJoin(df.queryExecution.executedPlan).isEmpty)
+    assert(findBroadcastNestedLoopJoin(df.queryExecution.executedPlan).size == 
1)
+  }
+
+  test("point-in-range matches the nested loop result") {
+    setupIpLookupViews()
+    val query = s"SELECT s.id, l.zip_id FROM signin_ip s JOIN ip_lookup l ON 
$pointCond"
+    val expected = sql(query).collect()
+    withRangeJoin() {
+      val df = sql(query)
+      assert(findRangeJoin(df.queryExecution.executedPlan).size == 1)
+      checkAnswer(df, expected)
+    }
+  }
+
+  test("BETWEEN is planned as a point-in-range join") {
+    setupIpLookupViews()
+    val query =
+      "SELECT s.id, l.zip_id FROM signin_ip s JOIN ip_lookup l " +
+        "ON s.ip_add_int BETWEEN l.BEGIN_IP2LONG AND l.END_IP2LONG"
+    val expected = sql(query).collect()
+    withRangeJoin(SQLConf.ALWAYS_INLINE_COMMON_EXPR.key -> "false") {
+      val df = sql(query)
+      val planned = findRangeJoin(df.queryExecution.executedPlan)
+      assert(planned.size == 1)
+      // BETWEEN lowers to one point expression. The other side keeps (low, 
high).
+      val keyCounts = Set(planned.head.leftKeys.length, 
planned.head.rightKeys.length)
+      assert(keyCounts == Set(1, 2))
+      checkAnswer(df, expected)
+    }
+  }
+
+  test("hints select range join, nested loop, or cartesian") {
+    setupIpLookupViews()
+    setupIntervalViews()
+    val expected = sql(
+      s"SELECT s.id, l.zip_id FROM signin_ip s JOIN ip_lookup l ON 
$pointCond").collect()
+    withRangeJoin() {
+      val broadcast = sql(

Review Comment:
   Added: Added regression tests for dual-hinted queries covering both inner 
joins (selecting `BuildRight`) and stream-preserving left outer joins 
(discarding unbuildable `BuildLeft`).



-- 
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]

Reply via email to