Why
Three functions each contain an exhaustive when over all 30 norm.generator.PgNodeExpression
subtypes that does nothing but enumerate children:
substituteGroupRteVars, GroupRteSubstitution.kt:78-134 (rebuilds each node with mapped children)
NodeTreeNullabilityAnalyzer.safetyWalkChildren, NodeTreeNullabilityAnalyzer.kt:520-558
NodeTreeNullabilityAnalyzer.containsVarOutsideRelation, NodeTreeNullabilityAnalyzer.kt:965-1022
Adding a subtype means editing three whens, and the KDoc on each spends paragraphs explaining how
its child coverage differs from the others. One structural definition of "children" on the sealed
type removes two of the three whens and turns the third into a three-line exception list.
Current state: child sets, per subtype
C = full structural children. Differences from C:
| Subtype |
substituteGroupRteVars |
safetyWalkChildren |
containsVarOutsideRelation |
Aggref |
arguments |
empty |
arguments |
GroupingFunc |
arguments |
empty |
arguments |
JsonExpr |
argument, onEmptyDefault, onErrorDefault |
empty |
argument, onEmptyDefault, onErrorDefault |
JsonConstructorExpr |
arguments only |
arguments + function |
arguments + function |
SubLink |
outerOperand |
outerOperand |
outerOperand |
CaseExpr |
all four lists/fields |
all four |
all four |
| everything else |
C |
C |
C |
So: containsVarOutsideRelation already walks C. safetyWalkChildren is C minus three
deliberate exclusions. substituteGroupRteVars is C except it never descends into
JsonConstructorExpr.function (the Aggref/WindowFunc behind JSON_OBJECTAGG/JSON_ARRAYAGG),
which is an omission, not a rule: its KDoc (lines 43-60) argues it should walk into aggregates
defensively, and it does so for a bare Aggref.
Target state
In PgNodeExpression.kt, two top-level members with a single exhaustive when each:
/** Every direct sub-expression, in serialization order. Leaf nodes return an empty list. */
internal val PgNodeExpression.children: List<PgNodeExpression>
get() = when (this) {
is PgNodeExpression.Var, is PgNodeExpression.Const, is PgNodeExpression.SqlValueFunction,
is PgNodeExpression.NextValExpr, is PgNodeExpression.Unknown -> emptyList()
is PgNodeExpression.FuncExpr -> arguments
// ... one branch per subtype, matching column C above ...
is PgNodeExpression.CaseExpr -> resultExpressions + listOfNotNull(defaultResult, testExpression) + whenConditions
is PgNodeExpression.JsonExpr -> listOfNotNull(argument, onEmptyDefault, onErrorDefault)
is PgNodeExpression.JsonConstructorExpr -> arguments + listOfNotNull(function)
is PgNodeExpression.SubLink -> listOfNotNull(outerOperand)
}
/** This node with [transform] applied to each direct child; leaf nodes return `this`. */
internal fun PgNodeExpression.mapChildren(transform: (PgNodeExpression) -> PgNodeExpression): PgNodeExpression =
when (this) {
is PgNodeExpression.Var, /* other leaves */ -> this
is PgNodeExpression.FuncExpr -> copy(arguments = arguments.map(transform))
is PgNodeExpression.CaseExpr -> copy(
resultExpressions = resultExpressions.map(transform),
defaultResult = defaultResult?.let(transform),
testExpression = testExpression?.let(transform),
whenConditions = whenConditions.map(transform),
)
is PgNodeExpression.JsonConstructorExpr -> copy(arguments = arguments.map(transform), function = function?.let(transform))
// ... etc.
}
Both whens have no else so a new subtype fails compilation here, and only here.
Then:
substituteGroupRteVars becomes: Var branch unchanged; otherwise
expression.mapChildren(recurse). Its KDoc paragraph on child coverage (lines 43-60) is replaced by
one sentence: "Walks every structural child via mapChildren."
containsVarOutsideRelation becomes: Var branch unchanged; otherwise
expression.children.any(recurse). Its KDoc paragraph about exhaustiveness (lines 950-959) shrinks
to a pointer at children.
safetyWalkChildren becomes:
private fun safetyWalkChildren(expression: PgNodeExpression): List<PgNodeExpression> = when (expression) {
is PgNodeExpression.Aggref, is PgNodeExpression.GroupingFunc, is PgNodeExpression.JsonExpr -> emptyList()
else -> expression.children
}
with a KDoc that states the three exclusions and why (aggregate/grouping nodes are terminal for
the domination check; JsonExpr is lossy-parsed and hardcoded unsafe by its callers).
The one behavior change, and why it is accepted
substituteGroupRteVars will now descend into JsonConstructorExpr.function. On PostgreSQL 18 a
WindowFunc inside JSON_ARRAYAGG(x) OVER (...) whose argument is a grouping key was previously
left as a bare GROUP-RTE Var (unresolved → "source column not found" → nullable). After this change
it is substituted exactly as the same WindowFunc outside a JSON constructor already is. This is the
16/17 parity the substitution exists to restore, and it can only move a result from nullable toward
the version-consistent answer, never the other way. It is called out here so nobody has to
rediscover it in a diff.
Test design
GroupRteSubstitutionTest.kt: add one test constructing a
JsonConstructorExpr(type = JSON_CONSTRUCTOR_TYPE_ARRAYAGG, arguments = emptyList(), function = WindowFunc(oid, listOf(Var(groupVarno, 1, emptySet())))) with a group-expression map resolving
(groupVarno, 1) to Var(1, 2, emptySet()), asserting the returned node's function argument is
the resolved Var. This pins the accepted behavior change.
NodeTreeNullabilityAnalyzerTest.kt: the existing containsVarOutsideRelation and grouping-set
safety tests pin the other two walkers. Add a test that children on each leaf subtype is empty
and that mapChildren(identity) returns an equal node for one representative of every non-leaf
subtype (a table-driven @ParameterizedTest over a list of sample nodes is enough). This is the
pin that the single when is complete.
QueryAnalysisTest and goldens: unchanged except that no golden should change either, since no
scenario uses JSON_*AGG ... OVER under GROUP BY on a GROUP-RTE key.
Acceptance criteria
git grep -n "is PgNodeExpression.XmlExpr" generator/src/main returns hits in exactly two
functions: children and mapChildren, plus the isNonNull / isSafeFromGroupingSetNullExtension
evaluators that genuinely branch on semantics. No child-enumeration when remains in
GroupRteSubstitution.kt, containsVarOutsideRelation, or safetyWalkChildren.
- Tests in "Test design" exist and pass.
./gradlew :generator:check :gradle-plugin:test passes; golden regeneration leaves
test-scenarios* unchanged.
Files
generator/src/main/kotlin/norm/generator/PgNodeExpression.kt
generator/src/main/kotlin/norm/generator/GroupRteSubstitution.kt
generator/src/main/kotlin/norm/generator/NodeTreeNullabilityAnalyzer.kt
generator/src/test/kotlin/norm/generator/GroupRteSubstitutionTest.kt
generator/src/test/kotlin/norm/generator/NodeTreeNullabilityAnalyzerTest.kt
Conventions every issue inherits
- Repo root:
/Volumes/Code/3rd-party/norm. Module under change is almost always generator/.
- Style: 2-space indent, 120 columns, full words in identifiers (
parameter, not param), no
section-separator comments, @Nested classes group tests. Format with ./gradlew spotlessApply.
- TDD: tests are written first or alongside. For a behavior-preserving refactor the existing suite is
the pin; each issue says which tests must also be added.
- Golden files under
test-scenarios*/ are never hand-edited. A refactor is behavior-preserving only
if ./gradlew :gradle-plugin:generateGoldenFiles leaves git status --porcelain test-scenarios test-scenarios-frameworks empty.
- Verification for a
generator/ change: ./gradlew :generator:check :gradle-plugin:test (Docker
required). Then the golden regeneration check above.
- Commit message explains the design decision, not the diff.
- Never run
./gradlew clean or disable the configuration/build cache to "fix" a build problem.
Why
Three functions each contain an exhaustive
whenover all 30norm.generator.PgNodeExpressionsubtypes that does nothing but enumerate children:
substituteGroupRteVars,GroupRteSubstitution.kt:78-134(rebuilds each node with mapped children)NodeTreeNullabilityAnalyzer.safetyWalkChildren,NodeTreeNullabilityAnalyzer.kt:520-558NodeTreeNullabilityAnalyzer.containsVarOutsideRelation,NodeTreeNullabilityAnalyzer.kt:965-1022Adding a subtype means editing three
whens, and the KDoc on each spends paragraphs explaining howits child coverage differs from the others. One structural definition of "children" on the sealed
type removes two of the three
whens and turns the third into a three-line exception list.Current state: child sets, per subtype
C= full structural children. Differences fromC:substituteGroupRteVarssafetyWalkChildrencontainsVarOutsideRelationAggrefGroupingFuncJsonExprJsonConstructorExprSubLinkCaseExprCCCSo:
containsVarOutsideRelationalready walksC.safetyWalkChildrenisCminus threedeliberate exclusions.
substituteGroupRteVarsisCexcept it never descends intoJsonConstructorExpr.function(theAggref/WindowFuncbehindJSON_OBJECTAGG/JSON_ARRAYAGG),which is an omission, not a rule: its KDoc (lines 43-60) argues it should walk into aggregates
defensively, and it does so for a bare
Aggref.Target state
In
PgNodeExpression.kt, two top-level members with a single exhaustivewheneach:Both
whens have noelseso a new subtype fails compilation here, and only here.Then:
substituteGroupRteVarsbecomes:Varbranch unchanged; otherwiseexpression.mapChildren(recurse). Its KDoc paragraph on child coverage (lines 43-60) is replaced byone sentence: "Walks every structural child via
mapChildren."containsVarOutsideRelationbecomes:Varbranch unchanged; otherwiseexpression.children.any(recurse). Its KDoc paragraph about exhaustiveness (lines 950-959) shrinksto a pointer at
children.safetyWalkChildrenbecomes:the domination check;
JsonExpris lossy-parsed and hardcoded unsafe by its callers).The one behavior change, and why it is accepted
substituteGroupRteVarswill now descend intoJsonConstructorExpr.function. On PostgreSQL 18 aWindowFuncinsideJSON_ARRAYAGG(x) OVER (...)whose argument is a grouping key was previouslyleft as a bare GROUP-RTE
Var(unresolved → "source column not found" → nullable). After this changeit is substituted exactly as the same
WindowFuncoutside a JSON constructor already is. This is the16/17 parity the substitution exists to restore, and it can only move a result from nullable toward
the version-consistent answer, never the other way. It is called out here so nobody has to
rediscover it in a diff.
Test design
GroupRteSubstitutionTest.kt: add one test constructing aJsonConstructorExpr(type = JSON_CONSTRUCTOR_TYPE_ARRAYAGG, arguments = emptyList(), function = WindowFunc(oid, listOf(Var(groupVarno, 1, emptySet()))))with a group-expression map resolving(groupVarno, 1)toVar(1, 2, emptySet()), asserting the returned node'sfunctionargument isthe resolved
Var. This pins the accepted behavior change.NodeTreeNullabilityAnalyzerTest.kt: the existingcontainsVarOutsideRelationand grouping-setsafety tests pin the other two walkers. Add a test that
childrenon each leaf subtype is emptyand that
mapChildren(identity)returns an equal node for one representative of every non-leafsubtype (a table-driven
@ParameterizedTestover a list of sample nodes is enough). This is thepin that the single
whenis complete.QueryAnalysisTestand goldens: unchanged except that no golden should change either, since noscenario uses
JSON_*AGG ... OVERunderGROUP BYon a GROUP-RTE key.Acceptance criteria
git grep -n "is PgNodeExpression.XmlExpr" generator/src/mainreturns hits in exactly twofunctions:
childrenandmapChildren, plus theisNonNull/isSafeFromGroupingSetNullExtensionevaluators that genuinely branch on semantics. No child-enumeration
whenremains inGroupRteSubstitution.kt,containsVarOutsideRelation, orsafetyWalkChildren../gradlew :generator:check :gradle-plugin:testpasses; golden regeneration leavestest-scenarios*unchanged.Files
generator/src/main/kotlin/norm/generator/PgNodeExpression.ktgenerator/src/main/kotlin/norm/generator/GroupRteSubstitution.ktgenerator/src/main/kotlin/norm/generator/NodeTreeNullabilityAnalyzer.ktgenerator/src/test/kotlin/norm/generator/GroupRteSubstitutionTest.ktgenerator/src/test/kotlin/norm/generator/NodeTreeNullabilityAnalyzerTest.ktConventions every issue inherits
/Volumes/Code/3rd-party/norm. Module under change is almost alwaysgenerator/.parameter, notparam), nosection-separator comments,
@Nestedclasses group tests. Format with./gradlew spotlessApply.the pin; each issue says which tests must also be added.
test-scenarios*/are never hand-edited. A refactor is behavior-preserving onlyif
./gradlew :gradle-plugin:generateGoldenFilesleavesgit status --porcelain test-scenarios test-scenarios-frameworksempty.generator/change:./gradlew :generator:check :gradle-plugin:test(Dockerrequired). Then the golden regeneration check above.
./gradlew cleanor disable the configuration/build cache to "fix" a build problem.