Skip to content

Commit 06058a7

Browse files
committed
feat: expand PostgreSQL aggregate calls and CTE query composition
Parse schema-qualified and quoted expression function names, DISTINCT/ALL aggregate arguments, in-call ORDER BY with direction and null placement, and WITHIN GROUP. Preserve these forms through FILTER/OVER and SQL emission. Represent aggregate ordering with its own AST node so literal sort expressions are parameterized as values instead of mistaken for SELECT output ordinals. Reject incomplete calls, malformed ordering, incompatible aggregate modifiers, and bare-star placements that PostgreSQL's grammar does not permit. Extend PostgreSQL CTEs with output-column lists, materialization hints, SELECT/TABLE/VALUES/compound bodies, and nested WITH queries. Share this query parser with scalar and derived subqueries, including LATERAL, and preserve CTE identifier source spelling and the existing body-first child layout. Emit full WITH clauses, preserving RECURSIVE and quoted names. Keep MySQL query parsing behavior and the 48-byte AST layout; append the new node kinds. Propagate subquery errors without dereferencing null ASTs. Reject unsupported aggregate and CTE semantics in the local engine before materialization, rather than skipping an unbuildable CTE and accidentally reading a same-named physical table. Nested/recursive CTE execution, column remapping, VALUES/TABLE execution, and materialization hints remain unsupported. DML CTEs, SEARCH/CYCLE and full PostgreSQL grammar parity remain outside this increment. Add parse/emission, malformed-input, parameterization, AST-shape and real executor shadowing regressions. Document supported forms and remaining gaps. Validation: forced make all/build-corpus-test passed 1,381 tests (37 external backend skips); 52 focused expression/AST sanitizer tests passed; 24 new original/emitted SQL pairs produced equivalent PostgreSQL ASTs after removing source positions. Independent review rechecked 20 grammar cases against the pinned oracle and all three physical-table shadowing reproductions. Benchmark harnesses, reports, samples and their README/ignore changes are intentionally excluded from this commit.
1 parent 4f601d2 commit 06058a7

15 files changed

Lines changed: 651 additions & 37 deletions

‎docs/postgresql-analysis-features.md‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ transformation. They support normal SQL emission and digest generation.
2121
PostgreSQL still performs name resolution, type checking, window inheritance
2222
validation and other semantic checks.
2323

24-
The new clauses remain bounded by the existing expression/subquery grammar.
25-
In particular, LATERAL currently uses the simple SELECT subquery callback;
26-
WITH/VALUES/compound derived-query operands and interval-valued frame offsets
27-
are not covered by this change.
24+
The new clauses remain bounded by the existing expression grammar. PostgreSQL
25+
subqueries now share compound-query parsing, including SELECT, TABLE, VALUES and
26+
WITH operands in derived tables and LATERAL. Interval-valued frame offsets remain
27+
outside the supported expression subset.
2828

2929
Window expressions previously disappeared from emitted SQL and digests because
3030
their AST nodes had no emitter handlers. They now appear correctly. Digest
@@ -72,6 +72,39 @@ position, interval prefix-literal qualifiers, or the remaining full expression
7272
and DDL grammar. These additions improve the native ParserSQL implementation;
7373
there is no PostgreSQL runtime fallback or production library dependency.
7474

75+
## Qualified calls, aggregates and CTEs
76+
77+
Expression calls accept schema-qualified names, including quoted components.
78+
Aggregate calls preserve `DISTINCT`/`ALL`, in-call `ORDER BY` (including direction
79+
and NULLS placement), and `WITHIN GROUP (ORDER BY ...)`. FILTER and OVER can
80+
follow the supported aggregate forms. This is syntactic parsing; PostgreSQL
81+
still checks aggregate signatures and semantic restrictions.
82+
83+
`NODE_FUNCTION_CALL` retains ordinary argument children and node-specific flags
84+
for qualification, DISTINCT, ALL and WITHIN GROUP. An appended
85+
`NODE_AGGREGATE_ORDER_BY` child contains `NODE_ORDER_BY_ITEM` children. Its sort
86+
expressions are values, not SELECT output ordinals: parameterization binds a
87+
literal `1` inside `array_agg(x ORDER BY 1)` but preserves the query-level
88+
`ORDER BY 1`. Malformed calls and missing closing delimiters report errors.
89+
90+
PostgreSQL CTE definitions accept output-column lists, `[NOT] MATERIALIZED`,
91+
SELECT/TABLE/VALUES/compound bodies and nested WITH queries. RECURSIVE is retained
92+
in the AST and emitted SQL; it does not enable recursive local execution.
93+
The definition body remains its first child, followed by `NODE_CTE_COLUMNS`
94+
when present. CTE names and column names preserve quoting. The CTE emitter
95+
reconstructs the complete WITH clause and main query.
96+
97+
The local engine rejects qualified calls, aggregate modifiers/ordering,
98+
recursive/nested CTEs, CTE output-column lists, materialization hints and
99+
VALUES/TABLE query bodies before CTE materialization. These additions do not extend CTE parameterization, which
100+
continues to return an unsupported-context error.
101+
102+
Remaining gaps include data-modifying CTEs and main statements, SEARCH/CYCLE,
103+
special EXTRACT/SUBSTRING forms, AT TIME ZONE, advanced grouping, and broader
104+
SQL/JSON, XML and DDL grammar. See PostgreSQL's
105+
[aggregate syntax](https://www.postgresql.org/docs/18/sql-expressions.html#SYNTAX-AGGREGATES)
106+
and [WITH queries](https://www.postgresql.org/docs/18/queries-with.html).
107+
75108
## Multiple statements
76109

77110
```cpp
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# PostgreSQL SELECT and CTE coverage increment
2+
3+
Continue the user-approved grammar expansion on PR #67. Execute inline, retain
4+
MySQL behavior and the 48-byte AST layout, and exclude all benchmark files from
5+
commits. Preserve syntax in ASTs and emitted SQL; reject newly parsed features
6+
in the local engine unless its existing implementation supports them.
7+
8+
1. Add failing PostgreSQL expression tests for qualified/quoted function names,
9+
DISTINCT/ALL aggregates, aggregate ORDER BY with direction/null placement,
10+
WITHIN GROUP, composition with FILTER/OVER, malformed argument boundaries,
11+
and parameterization of aggregate sort constants (which are not ordinals).
12+
2. Implement expression parsing and emission with node-specific flags and an
13+
appended aggregate order node. Retain ordinary function argument children.
14+
3. Add failing CTE tests for output columns, VALUES/TABLE/compound bodies,
15+
nested WITH, materialization hints, and strict delimiters. Preserve the body
16+
as the first definition child and explicit columns as a separate child.
17+
Add CTE emission and guard unsupported execution. DML CTEs remain deferred
18+
if their integration requires changes beyond this query-focused increment.
19+
4. Force rebuild and run full C++ tests, corpus harness, and PostgreSQL oracle
20+
comparisons. Inspect transitions before refreshing correctness snapshots.
21+
5. Obtain an independent code review, document behavior and remaining gaps,
22+
make detailed commits, push the branch, and update PR #67.
23+
24+
Review focus: quoted names, call versus column ambiguity, missing delimiters,
25+
aggregate order constants versus SELECT ordinals, CTE child layout and engine
26+
materialization behavior. Do not equate complete parsing with AST equivalence.
27+
28+
## Progress and review
29+
30+
- Qualified calls, aggregate modifiers/order and CTE composition implemented.
31+
Focused tests first demonstrated the missing syntax, then passed.
32+
- Ruling: extend the shared PostgreSQL subquery callback and its entry checks
33+
to support VALUES/TABLE/WITH in derived tables and scalar subqueries. This
34+
avoids different grammars for the same query in CTE and derived contexts.
35+
- Review exposed malformed EXISTS null dereference, quoted CTE name emission,
36+
incomplete operand acceptance in nested queries, duplicate bare WITH, ALL
37+
WITHIN GROUP acceptance, and aggregate star restrictions. Added regressions
38+
and corrected each against pinned PostgreSQL grammar.
39+
- Ruling: add NULLS FIRST/LAST in compound-query ordering because shared
40+
subquery parsing must preserve ordering clauses. Unsupported local execution
41+
remains explicitly rejected.
42+
- Review demonstrated physical-table shadowing when unsupported CTE bodies were
43+
skipped during execution. Reject VALUES/TABLE query bodies and nested CTE
44+
materialization before execution, with an executor regression test.
45+
- Final corpus replay: 27,014 complete parses (+771), no previously complete
46+
case lost. Gains: 638 SELECT, 123 EXPLAIN, nine UPDATE and one INSERT. Another
47+
79 trailing and 42 type-mismatch results now report explicit errors; these
48+
remain unsupported queries, not lost complete parses.
49+
- Forced full rebuild: 1,381 passing C++ tests, 37 backend skips; corpus harness
50+
built. Focused sanitizer run: 52 passing tests. All 24 new original/emitted
51+
PostgreSQL AST pairs match after source locations are removed.
52+
- Independent final review: no outstanding findings after 20 targeted grammar
53+
checks and three CTE physical-table shadowing reproductions.
54+
- Correctness snapshot refresh and PR publication follow the grammar commit.

‎include/sql_engine/plan_builder.h‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,36 +52,56 @@ class PlanBuilder {
5252
return nullptr;
5353
}
5454

55+
static bool supports_query_features(const sql_parser::AstNode* ast) {
56+
return ast && !has_unsupported_query_feature(ast);
57+
}
58+
5559
private:
5660
const Catalog& catalog_;
5761
sql_parser::Arena& arena_;
5862

59-
static bool has_unsupported_query_feature(const sql_parser::AstNode* node) {
63+
static bool has_unsupported_query_feature(const sql_parser::AstNode* node, bool root = true) {
6064
using sql_parser::NodeType;
6165
switch (node->type) {
66+
case NodeType::NODE_CTE:
67+
// Nested WITH requires its own materialization scope, which
68+
// neither build_cte nor the subquery executor implements.
69+
if (!root || (node->flags & sql_parser::FLAG_CTE_RECURSIVE)) return true;
70+
break;
71+
case NodeType::NODE_CTE_DEFINITION:
72+
if (node->flags & (sql_parser::FLAG_CTE_MATERIALIZED |
73+
sql_parser::FLAG_CTE_NOT_MATERIALIZED)) return true;
74+
break;
75+
case NodeType::NODE_CTE_COLUMNS:
76+
case NodeType::NODE_TABLE_QUERY:
77+
case NodeType::NODE_VALUES_CLAUSE:
78+
return true;
6279
case NodeType::NODE_BINARY_OP:
6380
case NodeType::NODE_UNARY_OP:
6481
if (node->flags & sql_parser::FLAG_PG_OPERATOR) return true;
6582
break;
6683
case NodeType::NODE_NAMED_ARGUMENT:
6784
case NodeType::NODE_TYPE_CAST:
6885
case NodeType::NODE_DISTINCT_ON:
86+
case NodeType::NODE_AGGREGATE_ORDER_BY:
6987
case NodeType::NODE_AGGREGATE_FILTER:
7088
case NodeType::NODE_LATERAL:
7189
case NodeType::NODE_WINDOW_CLAUSE:
7290
case NodeType::NODE_WINDOW_REFERENCE:
7391
case NodeType::NODE_WINDOW_FRAME:
7492
return true;
7593
case NodeType::NODE_FUNCTION_CALL:
76-
if (node->flags & sql_parser::FLAG_FUNCTION_TABLE) return true;
94+
if (node->flags & (sql_parser::FLAG_FUNCTION_TABLE | sql_parser::FLAG_FUNCTION_DISTINCT |
95+
sql_parser::FLAG_FUNCTION_ALL | sql_parser::FLAG_FUNCTION_QUALIFIED |
96+
sql_parser::FLAG_FUNCTION_WITHIN_GROUP)) return true;
7797
break;
7898
case NodeType::NODE_ORDER_BY_ITEM:
7999
if (node->flags & sql_parser::FLAG_ORDER_NULLS) return true;
80100
break;
81101
default: break;
82102
}
83103
for (const auto* child = node->first_child; child; child = child->next_sibling)
84-
if (has_unsupported_query_feature(child)) return true;
104+
if (has_unsupported_query_feature(child, false)) return true;
85105
return false;
86106
}
87107

‎include/sql_engine/plan_executor.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ class PlanExecutor {
115115
return execute(plan);
116116
}
117117

118+
// Reject unsupported CTE semantics before any materialization occurs.
119+
if (!PlanBuilder<D>::supports_query_features(ast)) return {};
120+
118121
// Materialize each CTE definition
119122
std::vector<std::unique_ptr<InMemoryDataSource>> cte_sources;
120123
for (const sql_parser::AstNode* child = ast->first_child; child; child = child->next_sibling) {

‎include/sql_parser/common.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,18 @@ static constexpr uint16_t FLAG_QUERY_PARENTHESIZED = 0x01;
6363
static constexpr uint16_t FLAG_TABLE_ONLY = 0x01;
6464
static constexpr uint16_t FLAG_TABLE_INHERIT = 0x02;
6565
static constexpr uint16_t FLAG_FUNCTION_TABLE = 0x01;
66+
static constexpr uint16_t FLAG_FUNCTION_DISTINCT = 0x02;
67+
static constexpr uint16_t FLAG_FUNCTION_ALL = 0x04;
68+
static constexpr uint16_t FLAG_FUNCTION_QUALIFIED = 0x08;
69+
static constexpr uint16_t FLAG_FUNCTION_WITHIN_GROUP = 0x10;
6670
// On binary/unary expressions: PostgreSQL operation without local engine support.
6771
static constexpr uint16_t FLAG_PG_OPERATOR = 0x01;
6872
static constexpr uint16_t FLAG_WINDOW_BETWEEN = 0x01;
6973
static constexpr uint16_t FLAG_ORDER_NULLS = 0x01;
7074
static constexpr uint16_t FLAG_LIMIT_COMMA = 0x01;
75+
static constexpr uint16_t FLAG_CTE_RECURSIVE = 0x01;
76+
static constexpr uint16_t FLAG_CTE_MATERIALIZED = 0x02;
77+
static constexpr uint16_t FLAG_CTE_NOT_MATERIALIZED = 0x04;
7178

7279
// -- Flags for NODE_IDENTIFIER / NODE_COLUMN_REF --
7380
// Set when the identifier was source-delimited (backtick `name` for MySQL,
@@ -267,6 +274,8 @@ enum class NodeType : uint16_t {
267274
NODE_TYPE_CAST, // expression, NODE_TYPE_NAME; canonical CAST emission
268275
NODE_TYPE_NAME, // validated type syntax, including modifiers/bounds
269276
NODE_NAMED_ARGUMENT, // value=argument name; one expression child
277+
NODE_AGGREGATE_ORDER_BY, // ORDER_BY_ITEM children; constants are not ordinals
278+
NODE_CTE_COLUMNS, // identifier children; follows body in CTE_DEFINITION
270279
};
271280

272281
} // namespace sql_parser

‎include/sql_parser/compound_query_parser.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,18 @@ class CompoundQueryParser {
210210
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
211211
}
212212

213+
if constexpr (D == Dialect::PostgreSQL) {
214+
if (ExpressionParser<D>::keyword(tok_.peek(), "NULLS")) {
215+
tok_.skip();
216+
Token placement = tok_.peek();
217+
bool first = ExpressionParser<D>::keyword(placement, "FIRST");
218+
if (!first && !ExpressionParser<D>::keyword(placement, "LAST")) return nullptr;
219+
tok_.skip();
220+
item->flags |= FLAG_ORDER_NULLS;
221+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
222+
first ? StringRef{"NULLS FIRST", 11} : StringRef{"NULLS LAST", 10}));
223+
}
224+
}
213225
order_by->add_child(item);
214226

215227
if (tok_.peek().type == TokenType::TK_COMMA) {

‎include/sql_parser/emitter.h‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ class Emitter {
5454
case NodeType::NODE_WHERE_CLAUSE: emit_where_clause(node); break;
5555
case NodeType::NODE_GROUP_BY_CLAUSE: emit_group_by(node); break;
5656
case NodeType::NODE_HAVING_CLAUSE: emit_having(node); break;
57-
case NodeType::NODE_ORDER_BY_CLAUSE: emit_order_by(node); break;
57+
case NodeType::NODE_ORDER_BY_CLAUSE:
58+
case NodeType::NODE_AGGREGATE_ORDER_BY: emit_order_by(node); break;
5859
case NodeType::NODE_ORDER_BY_ITEM: emit_order_by_item(node); break;
5960
case NodeType::NODE_LIMIT_CLAUSE: emit_limit(node); break;
6061
case NodeType::NODE_LOCKING_CLAUSE: emit_locking(node); break;
@@ -108,6 +109,9 @@ class Emitter {
108109
case NodeType::NODE_STAR_REPLACE: emit_star_replace(node); break;
109110
case NodeType::NODE_REPLACE_ITEM: emit_replace_item(node); break;
110111

112+
case NodeType::NODE_CTE: emit_cte(node); break;
113+
case NodeType::NODE_CTE_COLUMNS: emit_list(node, ", "); break;
114+
111115
// ---- Compound query ----
112116
case NodeType::NODE_COMPOUND_QUERY: emit_compound_query(node); break;
113117
case NodeType::NODE_SET_OPERATION: emit_set_operation(node); break;
@@ -1277,20 +1281,60 @@ class Emitter {
12771281
if (child) emit_node(child);
12781282
}
12791283

1284+
void emit_cte(const AstNode* node) {
1285+
sb_.append("WITH ");
1286+
if (node->flags & FLAG_CTE_RECURSIVE) sb_.append("RECURSIVE ");
1287+
const AstNode* child = node->first_child;
1288+
bool first = true;
1289+
for (; child && child->type == NodeType::NODE_CTE_DEFINITION; child = child->next_sibling) {
1290+
if (!first) sb_.append(", ");
1291+
first = false;
1292+
emit_identifier(child);
1293+
const AstNode* body = child->first_child;
1294+
const AstNode* columns = body ? body->next_sibling : nullptr;
1295+
if (columns && columns->type == NodeType::NODE_CTE_COLUMNS) {
1296+
sb_.append_char('(');
1297+
emit_node(columns);
1298+
sb_.append_char(')');
1299+
}
1300+
sb_.append(" AS ");
1301+
if (child->flags & FLAG_CTE_MATERIALIZED) sb_.append("MATERIALIZED ");
1302+
else if (child->flags & FLAG_CTE_NOT_MATERIALIZED) sb_.append("NOT MATERIALIZED ");
1303+
sb_.append_char('(');
1304+
emit_node(body);
1305+
sb_.append_char(')');
1306+
}
1307+
sb_.append_char(' ');
1308+
emit_node(child);
1309+
}
1310+
12801311
void emit_function_call(const AstNode* node) {
12811312
const AstNode* arg = node->first_child;
12821313
if (node->flags & FLAG_FUNCTION_TABLE) {
12831314
emit_node(arg);
12841315
arg = arg ? arg->next_sibling : nullptr;
12851316
} else emit_value(node);
12861317
sb_.append_char('(');
1318+
if (node->flags & FLAG_FUNCTION_DISTINCT) sb_.append("DISTINCT ");
1319+
else if (node->flags & FLAG_FUNCTION_ALL) sb_.append("ALL ");
12871320
bool first = true;
1321+
const AstNode* order = nullptr;
12881322
for (; arg; arg = arg->next_sibling) {
1323+
if (arg->type == NodeType::NODE_AGGREGATE_ORDER_BY) {
1324+
order = arg;
1325+
continue;
1326+
}
12891327
if (!first) sb_.append(", ");
12901328
first = false;
12911329
emit_node(arg);
12921330
}
1331+
if (order && !(node->flags & FLAG_FUNCTION_WITHIN_GROUP)) emit_node(order);
12931332
sb_.append_char(')');
1333+
if (order && (node->flags & FLAG_FUNCTION_WITHIN_GROUP)) {
1334+
sb_.append(" WITHIN GROUP (ORDER BY ");
1335+
emit_list(order, ", ");
1336+
sb_.append_char(')');
1337+
}
12941338
}
12951339

12961340
void emit_is_null(const AstNode* node) {

0 commit comments

Comments
 (0)