From 1753e9174592c0b53955630cb489c30171b2d6db Mon Sep 17 00:00:00 2001 From: russimicro Date: Wed, 29 Jul 2026 13:09:55 -0500 Subject: [PATCH] fix(sql): a two-table comma-join accepts a composite key `FROM a, b WHERE a.k1 = b.k1 AND a.k2 = b.k2` was rejected with "comma-join supports a single equality join key; use INNER JOIN ... ON for composite keys", while the identical predicate across THREE tables worked. The limit was never about composite keys -- it was about which executor ran: 2 tables -> the equality is lowered into stmt.inner_join, a JoinClause that holds exactly one left/right column pair 3+ tables -> the N-way executor, which reads the join equalities straight from the WHERE and handles any number of them So a two-table query hit a wall that a three-table one did not. Do not lower a composite key and do not reject it: leave inner_join unset, keep every equality in the WHERE, and let the N-way executor take the statement. The gate that selected it moves from `>= 3 tables` to `>= 3 tables, or 2 tables with no inner_join`. A two-table join with a single key still takes the simpler path, unchanged. Reported from an ERP whose historical movement query joins moviminv and conseinv on (ccodigocon, cdocumetra). Same failure on DBF/CDX and on ADT/ADI -- it is a parser limit, independent of storage format. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 7346b95b48d665e6d4861c47e59719f1f082f59d) --- src/abi/ace_exports.cpp | 8 ++++++- src/sql/parser.cpp | 40 +++++++++++++++++++++------------- tests/unit/sql_parser_test.cpp | 24 ++++++++++++++++++-- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/abi/ace_exports.cpp b/src/abi/ace_exports.cpp index 1ce9dc15..51073479 100644 --- a/src/abi/ace_exports.cpp +++ b/src/abi/ace_exports.cpp @@ -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; diff --git a/src/sql/parser.cpp b/src/sql/parser.cpp index 68396ded..1de96cd3 100644 --- a/src/sql/parser.cpp +++ b/src/sql/parser.cpp @@ -1761,22 +1761,32 @@ util::Result 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 [, ...] [HAVING ]. diff --git a/tests/unit/sql_parser_test.cpp b/tests/unit/sql_parser_test.cpp index 03583222..01aede98 100644 --- a/tests/unit/sql_parser_test.cpp +++ b/tests/unit/sql_parser_test.cpp @@ -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") {