Skip to content

Commit 5876b2d

Browse files
dantengskysundy-li
andauthored
fix(query): handle single-row inequality joins safely (#20155)
* fix(query): avoid range join for single-row side (#20062) * fix(query): clear single_to_inner for cross join fallback from range join When a scalar subquery inequality join (LeftSingle/RightSingle converted to Inner with single_to_inner marker) has no equi-conditions and falls back from RangeJoin to HashJoin, the hash join executes as a cross join + filter. The FROM_LEFT_SINGLE runtime check fires during the cross-product matching phase before the filter is applied, causing a false-positive "Scalar subquery can't return more than one row" error. Clear single_to_inner when the join has no equi-conditions so the runtime doesn't enforce it in this context. The scalar guarantee is already satisfied by construction (the aggregate always produces exactly one row). This is a follow-up to #20062 which introduced the single-row guard on both sides but did not handle the cross-join false-positive. * fix(query): preserve scalar guard for one-row outers * test(query): cover scalar probe cross join guard * test(query): remove duplicate scalar guard coverage * refactor(query): clarify scalar join cardinality wording * refactor(query): keep single-row helper concise * docs(query): clarify scalar-side cardinality comment * test(query): mask read cost in join explain --------- Co-authored-by: sundyli <543950155@qq.com>
1 parent e338ae4 commit 5876b2d

7 files changed

Lines changed: 204 additions & 5 deletions

File tree

src/query/service/src/physical_plans/physical_join.rs

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,30 @@ use crate::physical_plans::PhysicalPlanBuilder;
2727
use crate::physical_plans::explain::PlanStatsInfo;
2828
use crate::physical_plans::physical_plan::PhysicalPlan;
2929

30+
fn is_single_row(stat_info: &databend_common_sql::optimizer::ir::StatInfo) -> bool {
31+
matches!(stat_info.statistics.precise_cardinality, Some(1)) || stat_info.cardinality == 1.0
32+
}
33+
34+
fn is_precise_single_row(stat_info: &databend_common_sql::optimizer::ir::StatInfo) -> bool {
35+
matches!(stat_info.statistics.precise_cardinality, Some(1))
36+
}
37+
38+
fn single_join_scalar_side_is_precise_single_row(join: &Join, s_expr: &SExpr) -> Result<bool> {
39+
match join.single_to_inner {
40+
Some(JoinType::LeftSingle) => {
41+
let right_rel_expr = RelExpr::with_s_expr(s_expr.right_child());
42+
let right_stat_info = right_rel_expr.derive_cardinality()?;
43+
Ok(is_precise_single_row(&right_stat_info))
44+
}
45+
Some(JoinType::RightSingle) => {
46+
let left_rel_expr = RelExpr::with_s_expr(s_expr.left_child());
47+
let left_stat_info = left_rel_expr.derive_cardinality()?;
48+
Ok(is_precise_single_row(&left_stat_info))
49+
}
50+
_ => Ok(false),
51+
}
52+
}
53+
3054
enum PhysicalJoinType {
3155
Hash,
3256
// The first arg is range conditions, the second arg is other conditions
@@ -63,6 +87,7 @@ fn physical_join(join: &Join, s_expr: &SExpr) -> Result<PhysicalJoinType> {
6387

6488
let left_rel_expr = RelExpr::with_s_expr(s_expr.left_child());
6589
let right_rel_expr = RelExpr::with_s_expr(s_expr.right_child());
90+
let left_stat_info = left_rel_expr.derive_cardinality()?;
6691
let right_stat_info = right_rel_expr.derive_cardinality()?;
6792

6893
if !join.equi_conditions.is_empty() {
@@ -75,10 +100,10 @@ fn physical_join(join: &Join, s_expr: &SExpr) -> Result<PhysicalJoinType> {
75100
return Ok(PhysicalJoinType::Hash);
76101
}
77102

78-
if matches!(right_stat_info.statistics.precise_cardinality, Some(1))
79-
|| right_stat_info.cardinality == 1.0
80-
{
81-
// If the output rows of build side is equal to 1, we use CROSS JOIN + FILTER instead of RANGE JOIN.
103+
if is_single_row(&left_stat_info) || is_single_row(&right_stat_info) {
104+
// Prefer CROSS JOIN + FILTER when statistics prove or estimate one side at one row.
105+
// HashJoin remains correct if the estimate is wrong and avoids the result-block
106+
// overhead that RangeJoin can incur for this shape after join commutation.
82107
return Ok(PhysicalJoinType::Hash);
83108
}
84109

@@ -221,8 +246,25 @@ impl PhysicalPlanBuilder {
221246
} else {
222247
match physical_join(join, s_expr)? {
223248
PhysicalJoinType::Hash => {
249+
// When a LeftSingle/RightSingle join (scalar subquery) has no
250+
// equi-conditions, the hash join executes as a cross join + filter.
251+
// The single-join runtime check can fire during the cross-product
252+
// matching phase before the filter is applied. It is only safe to
253+
// clear the marker when the scalar side itself is proven to produce
254+
// exactly one row. A precise one-row cardinality on the other input
255+
// does not prove that.
256+
let join = if join.equi_conditions.is_empty()
257+
&& join.single_to_inner.is_some()
258+
&& single_join_scalar_side_is_precise_single_row(join, s_expr)?
259+
{
260+
let mut j = join.clone();
261+
j.single_to_inner = None;
262+
j
263+
} else {
264+
join.clone()
265+
};
224266
self.build_hash_join(
225-
join,
267+
&join,
226268
s_expr,
227269
required,
228270
others_required,

src/query/service/tests/it/sql/exec/range_join.rs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,122 @@ use databend_common_expression::types::number::NumberScalar;
1818
use databend_query::test_kits::TestFixture;
1919
use futures_util::TryStreamExt;
2020

21+
async fn explain_text(fixture: &TestFixture, query: &str) -> anyhow::Result<String> {
22+
let blocks = fixture
23+
.execute_query(&format!("EXPLAIN {query}"))
24+
.await?
25+
.try_collect::<Vec<DataBlock>>()
26+
.await?;
27+
let block = DataBlock::concat(&blocks).expect("concat explain blocks");
28+
let column = block.get_by_offset(0);
29+
let mut lines = Vec::with_capacity(block.num_rows());
30+
for row in 0..block.num_rows() {
31+
if let Some(ScalarRef::String(line)) = column.index(row) {
32+
lines.push(line.to_string());
33+
}
34+
}
35+
Ok(lines.join("\n"))
36+
}
37+
38+
fn extract_one_u64(blocks: Vec<DataBlock>) -> u64 {
39+
let block = DataBlock::concat(&blocks).expect("concat result blocks");
40+
assert_eq!(block.num_rows(), 1);
41+
match block.get_by_offset(0).index(0) {
42+
Some(ScalarRef::Number(NumberScalar::UInt64(v))) => v,
43+
other => panic!("unexpected count scalar: {other:?}"),
44+
}
45+
}
46+
47+
/// An inequality join with a precise-one-row aggregate side should use CROSS JOIN + FILTER
48+
/// instead of RangeJoin. Exercise both equivalent predicate operand orders and verify the
49+
/// selected physical operator and query result.
50+
#[tokio::test(flavor = "multi_thread")]
51+
async fn test_inequality_join_with_scalar_side_avoids_range_join() -> anyhow::Result<()> {
52+
let fixture = TestFixture::setup().await?;
53+
fixture.create_default_database().await?;
54+
let db = fixture.default_db_name();
55+
56+
fixture
57+
.execute_command(&format!(
58+
"CREATE TABLE {db}.big(ts INT) AS SELECT number FROM numbers(1000)"
59+
))
60+
.await?;
61+
fixture
62+
.execute_command(&format!(
63+
"CREATE TABLE {db}.threshold(ts INT) AS SELECT number FROM numbers(1000)"
64+
))
65+
.await?;
66+
67+
// Scalar subquery on the right side of the inequality.
68+
let q_right = format!(
69+
"SELECT count(*) FROM {db}.big b \
70+
WHERE b.ts >= (SELECT min(ts) FROM {db}.threshold)"
71+
);
72+
// Equivalent predicate with the scalar subquery written on the left.
73+
let q_left = format!(
74+
"SELECT count(*) FROM {db}.big b \
75+
WHERE (SELECT min(ts) FROM {db}.threshold) <= b.ts"
76+
);
77+
78+
for query in [&q_right, &q_left] {
79+
let plan = explain_text(&fixture, query).await?;
80+
assert!(
81+
!plan.contains("RangeJoin"),
82+
"single-row inequality side must not produce a RangeJoin, plan was:\n{plan}"
83+
);
84+
}
85+
86+
// Both forms must produce the correct result: all 1000 rows match `ts >= min(ts)`.
87+
for query in [&q_right, &q_left] {
88+
let blocks = fixture
89+
.execute_query(query)
90+
.await?
91+
.try_collect::<Vec<DataBlock>>()
92+
.await?;
93+
assert_eq!(extract_one_u64(blocks), 1000, "query: {query}");
94+
}
95+
96+
Ok(())
97+
}
98+
99+
/// Force the exact CROSS JOIN + FILTER shape that used to report a false scalar-subquery
100+
/// cardinality violation. The pushed-down expression is true for all four rows, but its
101+
/// estimated selectivity places the four-row table on the build side and the exact-one-row
102+
/// scalar aggregate on the probe side. The CROSS JOIN + FILTER execution must not treat the
103+
/// four cross-product candidates as four scalar-subquery rows.
104+
#[tokio::test(flavor = "multi_thread")]
105+
async fn test_scalar_aggregate_probe_cross_join_filter() -> anyhow::Result<()> {
106+
let fixture = TestFixture::setup().await?;
107+
fixture.create_default_database().await?;
108+
let db = fixture.default_db_name();
109+
110+
fixture
111+
.execute_command(&format!(
112+
"CREATE TABLE {db}.four_rows(ts INT) AS SELECT number FROM numbers(4)"
113+
))
114+
.await?;
115+
116+
let query = format!(
117+
"SELECT count(*) \
118+
FROM (SELECT ts FROM {db}.four_rows WHERE length(to_string(ts)) > 0) b \
119+
WHERE b.ts >= (SELECT min(number) FROM numbers(1))"
120+
);
121+
122+
let plan = explain_text(&fixture, &query).await?;
123+
assert!(plan.contains("HashJoin"), "plan was:\n{plan}");
124+
assert!(plan.contains("TableScan(Build)"), "plan was:\n{plan}");
125+
assert!(plan.contains("AggregateFinal(Probe)"), "plan was:\n{plan}");
126+
127+
let blocks = fixture
128+
.execute_query(&query)
129+
.await?
130+
.try_collect::<Vec<DataBlock>>()
131+
.await?;
132+
assert_eq!(extract_one_u64(blocks), 4);
133+
134+
Ok(())
135+
}
136+
21137
fn extract_two_u64(blocks: Vec<DataBlock>) -> (u64, u64) {
22138
let block = DataBlock::concat(&blocks).expect("concat blocks");
23139
assert_eq!(block.num_rows(), 1, "unexpected rows: {}", block.num_rows());

src/query/sql/src/planner/optimizer/optimizers/rule/join_rules/rule_commute_join.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ impl Rule for RuleCommuteJoin {
113113
(condition.right.clone(), condition.left.clone());
114114
}
115115
join.join_type = join.join_type.opposite();
116+
join.single_to_inner = join.single_to_inner.map(|join_type| join_type.opposite());
116117
let mut result = SExpr::create_binary(
117118
Arc::new(join.into()),
118119
Arc::new(right_child.clone()),

src/query/sql/src/planner/optimizer/optimizers/rule/join_rules/rule_commute_join_base_table.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ impl Rule for RuleCommuteJoinBaseTable {
9494
(condition.right.clone(), condition.left.clone());
9595
}
9696
join.join_type = join.join_type.opposite();
97+
join.single_to_inner = join.single_to_inner.map(|join_type| join_type.opposite());
9798
let mut result = SExpr::create_binary(
9899
Arc::new(join.into()),
99100
Arc::new(right_child.clone()),

src/query/sql/src/planner/optimizer/optimizers/rule/join_rules/rule_left_exchange_join.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ impl Rule for RuleLeftExchangeJoin {
102102
let t2 = s_expr.child(0)?.child(1)?;
103103
let t3 = s_expr.child(1)?;
104104

105+
// Reassociation does not preserve the scalar-cardinality semantics encoded by
106+
// single_to_inner, so do not exchange either marked join.
107+
if join1.single_to_inner.is_some() || join2.single_to_inner.is_some() {
108+
return Ok(());
109+
}
110+
105111
// Ensure inner joins or cross joins.
106112
if !matches!(join1.join_type, JoinType::Inner | JoinType::Cross)
107113
|| !matches!(join2.join_type, JoinType::Inner | JoinType::Cross)

tests/sqllogictests/suites/mode/standalone/explain/join.test

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,36 @@ HashJoin
10261026
├── push downs: [filters: [], limit: NONE]
10271027
└── estimated rows: 4.00
10281028

1029+
# The filter estimates the table side at zero rows, so join commutation places the
1030+
# precise-one-row aggregate on Probe. Keep this no-equi join on HashJoin, not RangeJoin.
1031+
query T
1032+
EXPLAIN SELECT * FROM (SELECT MAX(a) AS a FROM t2) AS s, t1 WHERE s.a <= t1.a AND t1.a > 100;
1033+
----
1034+
HashJoin
1035+
├── output columns: [MAX(a) (#1), t1.a (#2)]
1036+
├── join type: INNER
1037+
├── build keys: []
1038+
├── probe keys: []
1039+
├── keys is null equal: []
1040+
├── filters: [s.a (#1) <= t1.a (#2)]
1041+
├── estimated rows: 0.00
1042+
├── TableScan(Build)
1043+
│ ├── table: default.default.t1
1044+
│ ├── scan id: 1
1045+
│ ├── output columns: [a (#2)]
1046+
│ ├── read rows: 0
1047+
│ ├── read size: 0
1048+
│ ├── partitions total: 1
1049+
│ ├── partitions scanned: 0
1050+
│ ├── pruning stats: [segments: <read cost: <slt:ignore>, range pruning: 1 to 0 cost: <slt:ignore>>]
1051+
│ ├── push downs: [filters: [is_true(t1.a (#2) > 100)], limit: NONE]
1052+
│ └── estimated rows: 0.00
1053+
└── EvalScalar(Probe)
1054+
├── output columns: [MAX(a) (#1)]
1055+
├── expressions: [1]
1056+
├── estimated rows: 1.00
1057+
└── DummyTableScan
1058+
10291059
query T
10301060
EXPLAIN SELECT * FROM t1 JOIN t3 ON t1.a = t3.a;
10311061
----

tests/sqllogictests/suites/query/subquery.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,9 @@ insert into t1 values(1), (2), (3);
890890
query error 1001.*Scalar subquery can't return more than one row
891891
select (select sum(a) from t1 where t1.a >= t2.a group by t1.a) from t1 as t2;
892892

893+
query error 1001.*Scalar subquery can't return more than one row
894+
SELECT 1 WHERE (SELECT number FROM numbers(2)) >= 0;
895+
893896

894897

895898
statement ok

0 commit comments

Comments
 (0)