You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
- [x] FieldStore (composite field assignment wrapper that reuses the inner expression)
818
821
- [x] Float
819
822
- [x] FuncCall (comprehensive - basic function calls, special SQL standard functions with FROM/IN/PLACING syntax: EXTRACT, OVERLAY, POSITION, SUBSTRING, TRIM, TODO: WITHIN GROUP, FILTER)
823
+
- [x] FuncExpr (planner function invocation routed through the deparse bridge with placeholder `func#oid(...)` fallback)
824
+
- [x] FunctionParameter (CREATE FUNCTION parameters with mode keywords, identifiers, types, and DEFAULT clauses)
820
825
- [x] GrantStmt (GRANT/REVOKE privileges ON objects TO/FROM grantees, with options)
821
826
- [x] GrantRoleStmt (GRANT/REVOKE roles TO/FROM grantees WITH options GRANTED BY grantor)
822
827
- [x] GroupingFunc (GROUPING(columns) for GROUP BY GROUPING SETS)
- [x] XmlSerialize (XMLSERIALIZE(DOCUMENT/CONTENT expr AS type))
893
905
@@ -932,6 +944,17 @@ Keep this section focused on durable guidance. When you add new insights, summar
932
944
- Map `SelectStmt::limit_option` to `FETCH ... WITH TIES` when it resolves to `LimitOption::WithTies` so the re-parsed AST retains the original limit semantics.
933
945
- When wrapping a `SelectStmt` inside outer statements (e.g. VIEW, COPY), emit it via `emit_select_stmt_no_semicolon` so trailing clauses can follow before the final semicolon.
934
946
- Decode window frame bitmasks to render RANGE/ROWS/GROUPS with the correct UNBOUNDED/CURRENT/OFFSET bounds and guard PRECEDING/FOLLOWING against missing offsets.
947
+
- Ordered-set aggregates must render `WITHIN GROUP (ORDER BY ...)` outside the argument list and emit `FILTER (WHERE ...)` ahead of any `OVER` clause so planner fallbacks reuse the same surface layout.
948
+
949
+
**Planner Nodes (CRITICAL - Read Carefully)**:
950
+
- **NEVER create synthetic nodes or wrap nodes in SELECT statements for deparse round-trips**. This violates the architecture and breaks AST preservation.
951
+
- **NEVER call `pgt_query::deparse()` from emit functions**. The pretty printer must emit directly from AST nodes.
952
+
- Planner nodes (OpExpr, Aggref, WindowFunc, FuncExpr, SubPlan, etc.) represent internal PostgreSQL optimizer structures with OIDs instead of names.
953
+
- For planner nodes, emit simple fallback representations using OID placeholders (e.g., `op#123`, `func#456`, `agg#789`).
954
+
- Example: `OpExpr` with args `[a, b]` and `opno=96` emits as `a op#96 b` - we don't have operator symbols without a catalog lookup.
955
+
- `DistinctExpr` can emit `IS DISTINCT FROM` since the syntax is known; `NullIfExpr` can emit `NULLIF(a, b)` for the same reason.
956
+
- Planner nodes indicate the pretty printer was given optimizer output rather than parser output - the fallback representations are acceptable.
957
+
- When duplicating window frame logic between `WindowClause` and `WindowDef`, **copy and adapt the code directly** rather than creating synthetic nodes or calling helper functions that expect different node types.
935
958
936
959
### Logging Future Work
937
960
- Capture new learnings as concise bullets here and keep detailed session history in commit messages or external notes.
@@ -987,6 +1010,7 @@ just ready
987
1010
2. Spot-check MergeStmt WHEN clause formatting and add focused tests around mixed UPDATE/INSERT/DELETE branches if gaps appear.
988
1011
3. Audit existing TypeCast/TypeName snapshots for INTERVAL usages to confirm the new typmod decoding matches legacy expectations before broader review.
989
1012
4. Once the outstanding snapshot churn is cleared, re-run `cargo test -p pgt_pretty_print test_multi__window_60 -- --show-output` to confirm the refreshed ViewStmt emitter no longer diff's the window fixture.
1013
+
5. Add multi-statement coverage exercising ordered-set aggregates with FILTER clauses to validate planner fallbacks alongside the new single-statement fixture.
- Updated all affected nodes to emit fallback representations directly from their fields
20
+
21
+
**Learnings**:
22
+
-**NEVER create synthetic AST nodes or wrap nodes in SELECT for deparse round-trips** - this violates the architecture
23
+
-**NEVER call `pgt_query::deparse()` from emit functions** - the pretty printer must emit directly from AST nodes
24
+
- Planner nodes (OpExpr, Aggref, etc.) represent internal optimizer structures with OIDs; simple placeholder fallbacks are acceptable
25
+
- When duplicating logic between node types, copy and adapt code directly rather than creating synthetic nodes to reuse helpers
26
+
- The pretty printer is a pure AST-to-text emitter, not a parser round-tripper
27
+
28
+
**Next Steps**:
29
+
- Continue implementing remaining nodes using direct emission patterns
30
+
- Monitor for any other instances of synthetic node creation or deparse usage
31
+
- Keep the documentation updated with architectural constraints
32
+
---
33
+
34
+
---
35
+
**Date**: 2025-10-20 (Session 57)
36
+
**Nodes Implemented/Fixed**: FuncCall (WITHIN GROUP + FILTER)
37
+
**Progress**: 192/270 → 192/270
38
+
**Tests**: cargo test -p pgt_pretty_print test_single__func_call_within_group_filter_0_60 -- --show-output
39
+
**Key Changes**:
40
+
- Extended `func_call` emission to surface `WITHIN GROUP (ORDER BY ...)` and `FILTER (WHERE ...)` clauses with soft breakpoints ahead of windowing.
41
+
- Added a focused single-statement fixture and snapshot covering percentile aggregates with FILTER to guard the new output.
42
+
43
+
**Learnings**:
44
+
- Ordered-set aggregates store their ordering in `agg_order`; emitting it outside the argument list keeps both surface nodes and planner fallbacks aligned.
45
+
- FILTER clauses must precede any `OVER` window to mirror parser order and preserve AST equality.
46
+
47
+
**Next Steps**:
48
+
- Backfill a multi-statement regression that exercises ordered-set aggregates with FILTER to validate planner fallbacks under the shared emitter.
49
+
- Keep auditing `func_call` for remaining gaps such as VARIADIC support once current fixtures stabilise.
50
+
---
51
+
52
+
---
53
+
**Date**: 2025-10-20 (Session 56)
54
+
**Nodes Implemented/Fixed**: Aggref; WindowFunc
55
+
**Progress**: 190/270 → 192/270
56
+
**Tests**: cargo check -p pgt_pretty_print
57
+
**Key Changes**:
58
+
- Added dedicated `aggref` and `window_func` emitters that route planner-only nodes through the shared deparse bridge with defensive fallbacks.
59
+
- Registered both nodes in `mod.rs` so planner aggregates/windows no longer hit the dispatcher `todo!()`.
60
+
61
+
**Learnings**:
62
+
-`Aggref` and `WindowFunc` reparse into `FuncCall` trees, so keeping the shared function emitter feature-complete covers planner aggregates/windows too.
63
+
64
+
**Next Steps**:
65
+
- Teach `func_call` emission to surface FILTER/WITHIN GROUP clauses so deparse fallbacks stay faithful.
66
+
- Backfill targeted fixtures that exercise aggregate FILTER and OVER clauses with the new emitters.
67
+
---
68
+
69
+
---
70
+
**Date**: 2025-10-20 (Session 55)
71
+
**Nodes Implemented/Fixed**: FuncExpr
72
+
**Progress**: 189/270 → 190/270
73
+
**Tests**: cargo check -p pgt_pretty_print
74
+
**Key Changes**:
75
+
- Added a `func_expr` emitter that deparses planner-only function nodes back into surface syntax with a guarded placeholder fallback.
76
+
- Extended the shared deparse guard so planner calls that round-trip to `FuncExpr` do not recurse indefinitely.
77
+
- Inlined the `clear_location` helper into `sqljson_debug.rs` to restore `cargo check` after integrating the debug fixture.
78
+
79
+
**Learnings**:
80
+
- The synthetic `SELECT` deparse bridge handles `FuncExpr` without additional plumbing, keeping planner expressions aligned with surface emitters.
81
+
- Integration tests that live outside `tests/tests.rs` need a local copy of structural helpers until we centralise them in a shared module.
82
+
83
+
**Next Steps**:
84
+
- Bridge `Aggref` and `WindowFunc` planner nodes through the same deparse path to cover aggregate/window fixtures.
85
+
- Deduplicate the `clear_location` helper once we deliberately rework the test harness.
- Added an op_expr module that rehydrates planner-only operator nodes via libpg_query deparse before delegating back to the existing surface emitters.
95
+
- Wired SubPlan and AlternativeSubPlan through the same deparse bridge with guarded fallbacks to preserve formatting when deparse support is missing.
96
+
- Registered WithCheckOption emission so planner-enforced qualifiers no longer fall through the dispatcher.
97
+
98
+
**Learnings**:
99
+
- Wrapping planner nodes in a synthetic SELECT and round-tripping through libpg_query deparse reliably retrieves textual operators without hard-coding OID maps.
100
+
- Fallbacks should emit structurally valid SQL (even if degraded) to keep reparse checks happy when deparse cannot help.
101
+
102
+
**Next Steps**:
103
+
- Audit other planner-only nodes (e.g. FuncExpr, Alternative* wrappers) for the same deparse integration pattern.
104
+
- Consider caching the synthetic deparse ParseResult to avoid repeated allocations if performance becomes an issue.
- Exposed `emit_alter_table_cmd` and registered `NodeEnum::AlterTableCmd` so standalone ALTER TABLE commands format consistently with the aggregate statement helper.
114
+
- Promoted `emit_function_parameter` for reuse and wired `NodeEnum::FunctionParameter` into the dispatcher, aligning CREATE FUNCTION parameter rendering everywhere.
115
+
- Added a `window_clause` emitter that clones into `WindowDef` helpers and wrapped raw `WindowDef` nodes in their own layout group.
116
+
117
+
**Learnings**:
118
+
- Cloning protobuf window clauses into a temporary `WindowDef` keeps WINDOW definitions and OVER clauses in sync without duplicating frame bitmask logic.
119
+
- Many remaining planner nodes already have helper emitters embedded in statement modules; exposing them is often a matter of export + dispatcher wiring.
120
+
121
+
**Next Steps**:
122
+
- Continue wiring planner-only nodes such as `SubPlan`, `OpExpr`, and `WithCheckOption` to reduce `todo!` fallbacks.
123
+
- Investigate operator/OID lookup helpers needed for expression nodes before implementing the remaining arithmetic emitters.
- Replaced every remaining `TryFrom<i32>` conversion in node emitters with the prost-generated enum getters (`n.objtype()`, `cmd.subtype()`, etc.) to align with durable guidance and avoid silent fallbacks
133
+
- Simplified override handling in `InsertStmt` and join classification logic by leaning on typed getters; removed debug assertions that guarded integer enum misuse
134
+
- Added explicit `DropBehavior` matches where cascade handling is required so ALTER variants stay expressive without magic numbers
135
+
136
+
**Learnings**:
137
+
- Prost emits getter methods for each enum-backed field (e.g. `GrantStmt::targtype()`, `AlterTableCmd::behavior()`); using them keeps emitters concise and prevents drift when enum values change
138
+
- Auditing for leftover integer comparisons is easiest via `rg "try_from"` across `src/nodes`
139
+
140
+
**Next Steps**:
141
+
- Run `cargo clippy -p pgt_pretty_print` after the next batch of changes to ensure no regressions sneak back in
142
+
- Resume the pending WITH/RETURNING fixture integration work from Session 50 once ongoing formatting cleanups settle
143
+
---
144
+
9
145
---
10
146
**Date**: 2025-10-18 (Session 51)
11
147
**Nodes Implemented/Fixed**: Code quality improvements across all emit functions
0 commit comments