fix(sql-schema): stop generating a duplicate "id" column - #95
Open
albertoabellagarcia wants to merge 1 commit into
Open
fix(sql-schema): stop generating a duplicate "id" column#95albertoabellagarcia wants to merge 1 commit into
albertoabellagarcia wants to merge 1 commit into
Conversation
generate_sql_schema() appended a column for "id" twice in every generated schema: once via the main type-mapping branches (since an id property almost always declares its own "type", e.g. "string"), and again via the unconditional `if key == "id":` block at the end of the loop, which always fires regardless of what happened above. The result was an invalid CREATE TABLE statement declaring the same column twice in effectively every generated schema.sql. Moved the id special-case to the top of the loop with a `continue`, so it's handled exactly once and always as TEXT PRIMARY KEY, matching the original intent (id's own declared "type"/"format" was already being discarded in favor of TEXT PRIMARY KEY -- this just stops it from also being appended as a second column first). Verified against AirQualityObserved (the model from #87): "id" appears exactly once, correctly typed. Re-ran against all 231 real model.yaml files available locally: no exceptions, and exactly one "id" column per entity that declares an id property (3 entities in the set don't declare one at all, correctly unaffected). Co-Authored-By: Claude Sonnet 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.
Summary
Found while verifying #94 (
anyOfsupport, merged):generate_sql_schema()was generating a duplicate"id"column in effectively every schema it produces.idproperties almost always declare their own"type"(e.g."type": "string"), so the main type-mapping branch already appends a column forid. Then the unconditionalif key == "id":block at the end of the loop appendsidagain, this time asTEXT PRIMARY KEY-- with no check for whether it was already added above. Every generatedschema.sqlwith anidproperty ends up with something like:which is invalid SQL -- PostgreSQL rejects
CREATE TABLEstatements that declare the same column twice.Fix
Moved the
idspecial-case to the top of the loop, with acontinue:idis now handled exactly once and always typedTEXT PRIMARY KEY, which was already the effective outcome (the second append was overriding the first one's type) -- this just stops the first append from happening at all forid.Testing
AirQualityObserved(the model from generate_sql_schema - support for anyOf #87):"id"now appears exactly once in the generated schema, correctly typed asTEXT PRIMARY KEY.generate_sql_schema()against all 231 realmodel.yamlfiles available in a local FIWARE GitHub org checkout: no exceptions, and exactly one"id"column for every entity that declares anidproperty. 3 entities in that set (Distribution,Presentation) don't declare anidproperty at all and are correctly unaffected.Independent of #87/#94 -- this bug exists for any model with an
idproperty, regardless of whether it also usesanyOf.