Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/abi/ace_exports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27200,7 +27200,13 @@ static UNSIGNED32 exec_sql_direct_impl(ADSHANDLE hStatement, UNSIGNED8* pucSQL,
// two-table path). Pushing single-table predicates (e.g. the date range)
// down per level is a future optimisation; correctness first.
// ====================================================================
if (parsed.value().from_tables.size() >= 3) {
// Three or more tables always come here. Two tables come here only when
// the comma-join lowering declined -- a composite key, which the
// single-pair JoinClause cannot express (see parser.cpp). A two-table
// comma join with one key still takes the simpler path below.
if (parsed.value().from_tables.size() >= 3 ||
(parsed.value().from_tables.size() == 2 &&
!parsed.value().inner_join.has_value())) {
auto& st = parsed.value();
// S4 — aggregates (COUNT/SUM/AVG/MIN/MAX, optional GROUP BY /
// HAVING) are supported by accumulating during the join walk;
Expand Down
40 changes: 25 additions & 15 deletions src/sql/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1761,22 +1761,32 @@ util::Result<SelectStmt> parse_select(const std::string& sql) {
"comma-join requires an equality join predicate "
"(cartesian products are not supported)", sql};
}
if (keys.size() > 1) {
return util::Error{7200, 0,
"comma-join supports a single equality join key; "
"use INNER JOIN ... ON for composite keys", sql};
// A COMPOSITE key (more than one equality) cannot be expressed by the
// single-pair JoinClause. Do not lower it and do not reject it: leave
// inner_join unset and let the N-way executor take over -- it consumes
// from_tables + the WHERE equalities directly and handles any number
// of them, which is the route three or more tables already follow.
// Rejecting this was a two-table-only limit: `FROM a, b WHERE
// a.k1 = b.k1 AND a.k2 = b.k2` failed while the identical predicate
// across three tables worked.
//
// Only the single-key case is lowered, and only then is the predicate
// blanked out of the WHERE -- for a composite key every equality must
// stay in the WHERE, because that is where the N-way executor reads
// the join conditions from.
if (keys.size() == 1) {
WhereExpr* k = keys.front();
JoinClause j;
j.table = comma_join_table;
j.left_column = k->cmp.column;
j.right_column = k->cmp.outer_column;
stmt.inner_join = std::move(j);
// Blank the lifted predicate to an always-true empty-AND node.
k->cmp = WhereCmp{};
k->kind = WhereExpr::Kind::And;
k->children.clear();
k->child.reset();
}
WhereExpr* k = keys.front();
JoinClause j;
j.table = comma_join_table;
j.left_column = k->cmp.column;
j.right_column = k->cmp.outer_column;
stmt.inner_join = std::move(j);
// Blank the lifted predicate to an always-true empty-AND node.
k->cmp = WhereCmp{};
k->kind = WhereExpr::Kind::And;
k->children.clear();
k->child.reset();
}

// M10.25 — GROUP BY <col>[, <col>...] [HAVING <agg> <op> <num>].
Expand Down
24 changes: 22 additions & 2 deletions tests/unit/sql_parser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,30 @@ TEST_CASE("WHERE: ODBC date escape {d 'YYYY-MM-DD'} parses to digits") {
CHECK(r.value().where->cmp.literal == "20260101");
}

TEST_CASE("comma-join: composite (multiple) join keys are rejected") {
TEST_CASE("comma-join: a composite key parses and is left to the N-way path") {
// Used to be rejected outright ("use INNER JOIN ... ON for composite
// keys"). That was a two-table-only limit: the single-pair JoinClause
// cannot hold two key pairs, but the N-way executor reads the join
// equalities straight from the WHERE and handles any number of them --
// which is why the identical predicate across THREE tables always worked.
// Now the lowering simply declines: the statement parses, inner_join stays
// unset, and both equalities stay in the WHERE for the N-way executor.
auto r = parse_select(
"SELECT * FROM a, b WHERE a.x = b.y AND a.z = b.w");
CHECK_FALSE(r.has_value()); // single-key join only
REQUIRE(r.has_value());
CHECK_FALSE(r.value().inner_join.has_value());
CHECK(r.value().from_tables.size() == 2u);
CHECK(r.value().where != nullptr);
}

TEST_CASE("comma-join: a single key is still lowered into inner_join") {
// The simple path must not regress: one equality is still lifted out of
// the WHERE into the JoinClause.
auto r = parse_select(
"SELECT * FROM a, b WHERE a.x = b.y");
REQUIRE(r.has_value());
REQUIRE(r.value().inner_join.has_value());
CHECK(r.value().inner_join->table == "b");
}

TEST_CASE("comma-join: mixing comma with explicit JOIN is rejected") {
Expand Down
Loading