fix(server-core): use tablesSchemaV2 for Generate Data Model so PKs are detected - #11806
Open
haechangcho wants to merge 1 commit into
Open
Conversation
…re detected
/playground/db-schema and /playground/generate-schema both called
driver.tablesSchema(), which only queries information_schema.columns and
never merges primary/foreign key metadata. ScaffoldingSchema only marks a
dimension as a primary key via column.attributes.includes('primaryKey'),
which tablesSchema() never sets — so Generate Data Model never emits
primary_key: true for any table whose PK column isn't literally named `id`.
Any generated cube that also has a join then fails to compile with
"primary key for 'X' is required when join is defined".
driver.tablesSchemaV2() (BaseDriver) already exists for exactly this: it
calls tablesSchema() and merges in primaryKeys()/foreignKeys(), returning
the same DatabaseStructure shape with attributes/foreign_keys added where
applicable. Swapping both call sites to it is a backward-compatible
superset with no other behavior change.
Fixes cube-js#10517
github-actions
Bot
added
javascript
3 tasks
Author
|
@paveltiunov — since you've already looked closely at the manufacturing-KPI schema issue we filed (#11680), flagging this one too: this is a small, self-contained fix (2 lines) for #10517. Would appreciate a workflow approval so CI can actually run, and a look when you have a moment. Happy to make any changes requested. |
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
1 participant
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.
Check List
Issue Reference this PR resolves
#10517
Description of Changes Made (if issue reference is not provided)
/playground/db-schemaand/playground/generate-schemainDevServer.tsboth calleddriver.tablesSchema(), which only queriesinformation_schema.columnsand never setsattributes.ScaffoldingSchema.dimensions()decidesisPrimaryKeypurely fromcolumn.attributes?.includes('primaryKey'), so Generate Data Model never emittedprimary_key: trueunless a table's PK column happened to be namedid. Any generated cube that also had ajointhen failed to compile withprimary key for 'X' is required when join is defined.BaseDriver.tablesSchemaV2()already exists for exactly this (added in #8115) — it callstablesSchema()and merges inprimaryKeys()/foreignKeys(), returning the same shape withattributes/foreign_keysadded. It was defined but never actually called from any production code path; only a unit test exercised the PK-consuming logic directly with hand-builtattributesdata, so the missing wiring went unnoticed.Swapped both call sites to
tablesSchemaV2()— a backward-compatible superset, no other behavior change.Notes
driveris typed asBaseDriver(@cubejs-backend/query-orchestrator), which declarestablesSchemaV2()publicly, so no capability guard is needed.DevServer.ts's route wiring directly; the PK-detection logic itself (attributes.includes('primaryKey')→isPrimaryKey) is already covered byscaffolding-schema.test.ts/scaffolding-template.test.ts.