templates: one mechanism owns the numbering partition default (#7147) - #7200
Merged
Conversation
#7101's create-path allocator resolved a null `per:` FK to its own copy of the relation's `init:` (`numberPerDefault`), on the premise that "the database applies that on insert". Since #7104/#7115 the applyDefaults macro is the FIRST statement of `save()` and assigns exactly that FK from exactly that value, so by the time the numbering block runs the FK is already set and the `== null` arm is unreachable. Both arms resolved the same partition, so nothing was wrong at run time - but the comment documented a premise that no longer holds, and a reader would take a dead fallback for the load-bearing guarantee. The arm and the marker are gone: the allocator reads the FK the write has already defaulted, and a null FK now means what it says - a relation that declares no `init:` at all, i.e. the tenant-wide base row. `numberPerDefault` had exactly one consumer, so `EdmIntentGenerator.putNumberPartitionDefaults` goes with it (which also reunites the `ProcessIds` javadoc it had been inserted in front of with `processIdsProperty`). The issue's second half asked for the partition default to be unquoted like `#defaultLiteral`, so an `init: 'ACME'` could not allocate in partition `'ACME'` beside `ACME`. It cannot: unlike a field `default:`, a relation's `init:` is resolved by `StatusSymbolResolver` against the target's own seeds and anything that is neither a seeded name nor a numeric id is refused at parse - so the quoted shape never reaches a generator. Adding the unquoting would have been a second piece of dead code beside the one being removed, so the invariant is stated where a reader would otherwise wonder and pinned by a test instead. The `stampOn: issue` stamp keeps its own `perDefault` fallback: it reads a row loaded from the database, where the column can genuinely hold null (an explicit null write into a nullable column with a DEFAULT stores null). Verified: the rendered repository defaults the FK before the allocation and carries no second copy of the init value (IntentEngineIT's numbering case, rewritten to assert that ORDER rather than the old ternary constant, green end to end); engine-intent's 1182 unit tests green. Fixes #7147 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7147
What was wrong
#7101's create-path allocator resolved a null
per:FK to its own copy of the relation'sinit:(numberPerDefault), on the premise that "a partition FK the caller left unset is still null here even when the relation carries aninit:default (the database applies that on insert)". Since #7104/#7115 theapplyDefaultsmacro is the first statement ofsave()and assigns exactly that FK from exactly that value — so by the time the numbering block runs the FK is already set and the== nullarm is unreachable.Both arms resolved the same partition, so nothing was wrong at run time. What was wrong is the documentation: the comment stated a premise that no longer holds, and a reader would take a dead fallback for the load-bearing guarantee.
What changed
init:at all, i.e. the tenant-wide base row. The comment now points at the mechanism that actually owns the value.numberPerDefaulthad exactly one consumer, soEdmIntentGenerator.putNumberPartitionDefaultsgoes with it. (Side benefit: it had been inserted between theProcessIdsjavadoc andprocessIdsProperty; removing it reunites them.)stampOn: issuestamp keeps its ownperDefaultfallback. That path reads a row loaded from the database, where the column can genuinely hold null — an explicit null write into a nullable column carrying a DEFAULT stores null — so its fallback is live, not dead.On the second half of the issue
The issue asked for the partition default to be unquoted like
#defaultLiteral, so aninit: 'ACME'could not allocate in partition'ACME'besideACME. It cannot happen, and not only because intent PKs are integer-only today: unlike a fielddefault:, a relation'sinit:is resolved byStatusSymbolResolveragainst the target's own seeds, and anything that is neither a seeded name nor a numeric id is refused at parse. So the quoted authoring shape never reaches a generator, and adding the unquoting would have been a second piece of dead code beside the one this PR removes.The invariant is therefore stated where a reader would otherwise wonder (
NumberingSupport.partitionDefault's javadoc) and pinned by a test asserting the parse refusal, rather than defended by unreachable code. Happy to add the unquoting anyway if you'd rather have the belt.Verification
IntentEngineIT#numbering_partitions_the_default_company_by_the_relations_initrewritten to assert that ORDER rather than the old ternary constant, run end to end (green).engine-intent: 1182 unit tests green.EdmPartitionedNumberInitTestnow asserts the .model half of the guarantee (the FK's owndataDefaultValue, and that the number carries no duplicate).formatter:validateclean, javadoc clean under-P release.Generated output for an existing model is unchanged except for the removed branch, which was never taken.
🤖 Generated with Claude Code