Remove INT ID column in DB Schema#1547
Conversation
KaveeshaPiumini
requested review from
ThaminduDilshan and
darshanasbg
as code owners
KaveeshaPiumini
added
Type/Improvement
breaking change
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved surrogate integer/autoincrement Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1547 +/- ##
=======================================
Coverage 91.35% 91.35%
=======================================
Files 692 692
Lines 46194 46194
=======================================
Hits 42202 42202
Misses 2673 2673
Partials 1319 1319
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
We are just removing the ID column, Then the THEME table has THEME_ID as the primary key. I feel like we can rename them to ID as well. |
KaveeshaPiumini
force-pushed
the
thunder
branch
from
2937e58 to
8599529
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/dbscripts/runtimedb/postgres.sql (1)
2-12:⚠️ Potential issue | 🟠 MajorInconsistent PK pattern: single-column PK on
CODE_IDwhile all other tables in this file use composite PKs withDEPLOYMENT_ID.Every other table in this file (
AUTHORIZATION_REQUEST,FLOW_CONTEXT,FLOW_USER_DATA,WEBAUTHN_SESSION) usesPRIMARY KEY (X_ID, DEPLOYMENT_ID). This table now usesCODE_IDalone as the PK, breaking the established multi-deployment composite-key convention.While UUIDv7 guarantees global uniqueness, making the single-column PK functionally correct, this inconsistency has two consequences:
- Redundant UNIQUE constraint —
UNIQUE (CODE_ID, DEPLOYMENT_ID)on Line 11 adds no value sinceCODE_IDis already unique as the PK, and no child table references this pair.- Pattern divergence — Queries, store-layer code, and future maintainers will need to handle two different PK styles within the same database.
Consider using
PRIMARY KEY (CODE_ID, DEPLOYMENT_ID)for consistency (dropping the now-redundant UNIQUE), or explicitly document why this table deviates.Based on learnings: "All tables must support multi-deployment scenarios by including
DEPLOYMENT_IDas part of the composite primary key."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/runtimedb/postgres.sql` around lines 2 - 12, The AUTHORIZATION_CODE table uses a single-column primary key on CODE_ID which breaks the file's composite PK convention (DEPLOYMENT_ID + id) and makes the existing UNIQUE (CODE_ID, DEPLOYMENT_ID) redundant; change the primary key to PRIMARY KEY (CODE_ID, DEPLOYMENT_ID) (i.e., include DEPLOYMENT_ID in the PK definition for AUTHORIZATION_CODE) and remove the redundant UNIQUE constraint so the table matches other tables (AUTHORIZATION_REQUEST, FLOW_CONTEXT, FLOW_USER_DATA, WEBAUTHN_SESSION) that include DEPLOYMENT_ID in their composite keys.backend/dbscripts/userdb/postgres.sql (1)
2-15:⚠️ Potential issue | 🟠 MajorSame PK inconsistency as
AUTHORIZATION_CODE: single-column PKs onOU_ID,USER_ID,GROUP_IDinstead of composite PKs withDEPLOYMENT_ID.The same pattern divergence flagged in
runtimedb/postgres.sqlapplies here. However, unlikeAUTHORIZATION_CODE, theUNIQUE (X_ID, DEPLOYMENT_ID)constraints on these tables are still justified as FK reference targets forGROUP_MEMBER_REFERENCE(Line 60) andUSER_INDEXED_ATTRIBUTES(Line 75).If these tables are switched to
PRIMARY KEY (X_ID, DEPLOYMENT_ID)for consistency, the separate UNIQUE constraints can be dropped since the composite PK will serve as the FK target.Based on learnings: "All tables must support multi-deployment scenarios by including
DEPLOYMENT_IDas part of the composite primary key."Also applies to: 20-31, 36-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/userdb/postgres.sql` around lines 2 - 15, Change single-column primary keys to composite primary keys including DEPLOYMENT_ID for multi-deployment support: replace PRIMARY KEY on OU_ID (and similarly USER_ID, GROUP_ID / tables referenced in the comment) with PRIMARY KEY (OU_ID, DEPLOYMENT_ID) so DEPLOYMENT_ID is part of the PK; remove the now-redundant UNIQUE (OU_ID, DEPLOYMENT_ID) constraints since the composite PK provides uniqueness; ensure foreign key targets (e.g., GROUP_MEMBER_REFERENCE and USER_INDEXED_ATTRIBUTES) reference the composite PK columns accordingly and apply the same PK change to the other tables noted (the blocks referenced at lines 20-31 and 36-46).backend/dbscripts/thunderdb/postgres.sql (1)
197-211:⚠️ Potential issue | 🟡 Minor
RESOURCE_SERVERandACTIONstill retainINT IDcolumns — these should be removed.Per the PR's stated criteria (removing INT IDs "where they were not used by implementation"), the store layer uses string UUIDs exclusively:
resourceStoreInterfacedefines methods withresServerID stringandresID *stringparameters- All SQL queries reference
RESOURCE_SERVER_IDandRESOURCE_ID(VARCHAR columns with composite keys), never the INTIDcolumn- Model structs (
ResourceServer,Action,Resource) all use string identifiersThe INT
IDcolumns are not referenced by any part of the implementation and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/postgres.sql` around lines 197 - 211, Remove the unused integer surrogate ID columns from the RESOURCE_SERVER, ACTION, and RESOURCE table definitions: delete the "ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY" lines and instead declare the existing string identifier columns as the table key/unique constraint used by the application (e.g., keep or convert UNIQUE (RESOURCE_SERVER_ID, DEPLOYMENT_ID) to the primary identifier for RESOURCE_SERVER, and likewise ensure ACTION and RESOURCE rely on RESOURCE_ID/RESOURCE_SERVER_ID and DEPLOYMENT_ID string columns). Update any PRIMARY KEY/UNIQUE declarations so the existing VARCHAR UUID columns (RESOURCE_SERVER_ID, RESOURCE_ID, DEPLOYMENT_ID, etc.) enforce uniqueness as required by the implementation and remove references to the removed ID column.
♻️ Duplicate comments (3)
backend/dbscripts/userdb/sqlite.sql (1)
2-15: Same PK pattern issue as the PostgreSQL counterpart — single-column PKs withoutDEPLOYMENT_ID.The SQLite schema mirrors the PostgreSQL
userdbchanges faithfully, with the same single-column PK pattern onOU_ID,USER_ID, andGROUP_ID. The same recommendation to use composite PKs withDEPLOYMENT_IDapplies here.Also applies to: 20-31, 36-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/userdb/sqlite.sql` around lines 2 - 15, The ORGANIZATION_UNIT table (and the other SQLite tables using single-column PKs such as the ones with USER_ID and GROUP_ID) currently declares OU_ID as PRIMARY KEY without including DEPLOYMENT_ID, which breaks the multi-tenant composite PK pattern; update the CREATE TABLE for ORGANIZATION_UNIT to make the primary key composite (OU_ID, DEPLOYMENT_ID) and remove or adjust the separate UNIQUE (OU_ID, DEPLOYMENT_ID) clause accordingly, and apply the same change to the corresponding USER and GROUP table definitions so their PKs include DEPLOYMENT_ID (e.g., change PRIMARY KEY from USER_ID to (USER_ID, DEPLOYMENT_ID) and from GROUP_ID to (GROUP_ID, DEPLOYMENT_ID)).backend/dbscripts/thunderdb/postgres.sql (1)
4-4: Same PK inconsistency across all eight changed tables — single-column PKs withoutDEPLOYMENT_ID.All eight tables (
USER_SCHEMAS,ROLE,THEME,LAYOUT,APPLICATION,IDP,NOTIFICATION_SENDER,CERTIFICATE) use single-column PKs on their UUID columns. TheUNIQUE (X_ID, DEPLOYMENT_ID)constraints are still justified here as FK reference targets (e.g.,ROLE_PERMISSION→ROLE,APPLICATION→THEME/LAYOUT, etc.), but the PK-level inconsistency with the composite-key pattern remains.Also applies to: 21-21, 80-80, 95-95, 110-110, 150-150, 167-167, 183-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/postgres.sql` at line 4, The tables use single-column primary keys (e.g., SCHEMA_ID in USER_SCHEMAS, ROLE_ID in ROLE, THEME_ID in THEME, LAYOUT_ID in LAYOUT, APPLICATION_ID in APPLICATION, IDP_ID in IDP, NOTIFICATION_SENDER_ID in NOTIFICATION_SENDER, CERTIFICATE_ID in CERTIFICATE) while related FKs and UNIQUE constraints expect composite keys with DEPLOYMENT_ID; update each table definition to make the primary key composite by including DEPLOYMENT_ID in the PK (e.g., PRIMARY KEY (SCHEMA_ID, DEPLOYMENT_ID)), keep or adjust the existing UNIQUE (X_ID, DEPLOYMENT_ID) constraints as needed, and ensure any referencing foreign keys (e.g., ROLE_PERMISSION → ROLE, APPLICATION → THEME/LAYOUT) target the composite key columns.backend/dbscripts/thunderdb/sqlite.sql (1)
4-4: Same PK pattern as the PostgreSQL counterpart — consistent withthunderdb/postgres.sqlchanges.The SQLite schema correctly mirrors all eight table changes from the PostgreSQL version. The same single-column PK observation applies.
Also applies to: 21-21, 79-79, 94-94, 109-109, 149-149, 166-166, 182-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/sqlite.sql` at line 4, Several tables in this SQLite schema define a single-column primary key (e.g., "SCHEMA_ID VARCHAR(36) PRIMARY KEY") which diverges from the PostgreSQL changes — update each affected table to use the exact same primary key definitions used in thunderdb/postgres.sql so the schemas match; locate occurrences like SCHEMA_ID VARCHAR(36) PRIMARY KEY (and the equivalents at the other listed locations) and replace them with the identical PK clause from the Postgres file (including any composite keys or named constraints), then validate the SQLite DDL to ensure the PKs are applied correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/dbscripts/runtimedb/postgres.sql`:
- Around line 2-12: The AUTHORIZATION_CODE table uses a single-column primary
key on CODE_ID which breaks the file's composite PK convention (DEPLOYMENT_ID +
id) and makes the existing UNIQUE (CODE_ID, DEPLOYMENT_ID) redundant; change the
primary key to PRIMARY KEY (CODE_ID, DEPLOYMENT_ID) (i.e., include DEPLOYMENT_ID
in the PK definition for AUTHORIZATION_CODE) and remove the redundant UNIQUE
constraint so the table matches other tables (AUTHORIZATION_REQUEST,
FLOW_CONTEXT, FLOW_USER_DATA, WEBAUTHN_SESSION) that include DEPLOYMENT_ID in
their composite keys.
In `@backend/dbscripts/thunderdb/postgres.sql`:
- Around line 197-211: Remove the unused integer surrogate ID columns from the
RESOURCE_SERVER, ACTION, and RESOURCE table definitions: delete the "ID INT
GENERATED ALWAYS AS IDENTITY PRIMARY KEY" lines and instead declare the existing
string identifier columns as the table key/unique constraint used by the
application (e.g., keep or convert UNIQUE (RESOURCE_SERVER_ID, DEPLOYMENT_ID) to
the primary identifier for RESOURCE_SERVER, and likewise ensure ACTION and
RESOURCE rely on RESOURCE_ID/RESOURCE_SERVER_ID and DEPLOYMENT_ID string
columns). Update any PRIMARY KEY/UNIQUE declarations so the existing VARCHAR
UUID columns (RESOURCE_SERVER_ID, RESOURCE_ID, DEPLOYMENT_ID, etc.) enforce
uniqueness as required by the implementation and remove references to the
removed ID column.
In `@backend/dbscripts/userdb/postgres.sql`:
- Around line 2-15: Change single-column primary keys to composite primary keys
including DEPLOYMENT_ID for multi-deployment support: replace PRIMARY KEY on
OU_ID (and similarly USER_ID, GROUP_ID / tables referenced in the comment) with
PRIMARY KEY (OU_ID, DEPLOYMENT_ID) so DEPLOYMENT_ID is part of the PK; remove
the now-redundant UNIQUE (OU_ID, DEPLOYMENT_ID) constraints since the composite
PK provides uniqueness; ensure foreign key targets (e.g., GROUP_MEMBER_REFERENCE
and USER_INDEXED_ATTRIBUTES) reference the composite PK columns accordingly and
apply the same PK change to the other tables noted (the blocks referenced at
lines 20-31 and 36-46).
---
Duplicate comments:
In `@backend/dbscripts/thunderdb/postgres.sql`:
- Line 4: The tables use single-column primary keys (e.g., SCHEMA_ID in
USER_SCHEMAS, ROLE_ID in ROLE, THEME_ID in THEME, LAYOUT_ID in LAYOUT,
APPLICATION_ID in APPLICATION, IDP_ID in IDP, NOTIFICATION_SENDER_ID in
NOTIFICATION_SENDER, CERTIFICATE_ID in CERTIFICATE) while related FKs and UNIQUE
constraints expect composite keys with DEPLOYMENT_ID; update each table
definition to make the primary key composite by including DEPLOYMENT_ID in the
PK (e.g., PRIMARY KEY (SCHEMA_ID, DEPLOYMENT_ID)), keep or adjust the existing
UNIQUE (X_ID, DEPLOYMENT_ID) constraints as needed, and ensure any referencing
foreign keys (e.g., ROLE_PERMISSION → ROLE, APPLICATION → THEME/LAYOUT) target
the composite key columns.
In `@backend/dbscripts/thunderdb/sqlite.sql`:
- Line 4: Several tables in this SQLite schema define a single-column primary
key (e.g., "SCHEMA_ID VARCHAR(36) PRIMARY KEY") which diverges from the
PostgreSQL changes — update each affected table to use the exact same primary
key definitions used in thunderdb/postgres.sql so the schemas match; locate
occurrences like SCHEMA_ID VARCHAR(36) PRIMARY KEY (and the equivalents at the
other listed locations) and replace them with the identical PK clause from the
Postgres file (including any composite keys or named constraints), then validate
the SQLite DDL to ensure the PKs are applied correctly.
In `@backend/dbscripts/userdb/sqlite.sql`:
- Around line 2-15: The ORGANIZATION_UNIT table (and the other SQLite tables
using single-column PKs such as the ones with USER_ID and GROUP_ID) currently
declares OU_ID as PRIMARY KEY without including DEPLOYMENT_ID, which breaks the
multi-tenant composite PK pattern; update the CREATE TABLE for ORGANIZATION_UNIT
to make the primary key composite (OU_ID, DEPLOYMENT_ID) and remove or adjust
the separate UNIQUE (OU_ID, DEPLOYMENT_ID) clause accordingly, and apply the
same change to the corresponding USER and GROUP table definitions so their PKs
include DEPLOYMENT_ID (e.g., change PRIMARY KEY from USER_ID to (USER_ID,
DEPLOYMENT_ID) and from GROUP_ID to (GROUP_ID, DEPLOYMENT_ID)).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/dbscripts/runtimedb/postgres.sqlbackend/dbscripts/runtimedb/sqlite.sqlbackend/dbscripts/thunderdb/postgres.sqlbackend/dbscripts/thunderdb/sqlite.sqlbackend/dbscripts/userdb/postgres.sqlbackend/dbscripts/userdb/sqlite.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/dbscripts/runtimedb/sqlite.sql
KaveeshaPiumini
force-pushed
the
thunder
branch
from
8599529 to
64f9c3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/dbscripts/runtimedb/postgres.sql (1)
4-11:⚠️ Potential issue | 🟠 MajorPrimary key now enforces global uniqueness instead of deployment-scoped uniqueness.
Line 4 makes
CODE_IDglobally unique, while Line 11 indicates prior intent to scope uniqueness withDEPLOYMENT_ID. This is a behavioral schema change (not just PK promotion) and can break data compatibility across deployments. Prefer a composite PK on(CODE_ID, DEPLOYMENT_ID)and drop the redundant unique constraint.Proposed DDL fix
CREATE TABLE AUTHORIZATION_CODE ( DEPLOYMENT_ID VARCHAR(255) NOT NULL, - CODE_ID VARCHAR(36) PRIMARY KEY, + CODE_ID VARCHAR(36) NOT NULL, AUTHORIZATION_CODE VARCHAR(500) NOT NULL, CLIENT_ID VARCHAR(255) NOT NULL, STATE VARCHAR(50) NOT NULL, AUTHZ_DATA JSONB NOT NULL, TIME_CREATED TIMESTAMP NOT NULL, EXPIRY_TIME TIMESTAMP NOT NULL, - UNIQUE (CODE_ID, DEPLOYMENT_ID) + PRIMARY KEY (CODE_ID, DEPLOYMENT_ID) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/runtimedb/postgres.sql` around lines 4 - 11, The current schema makes CODE_ID the primary key, making it globally unique; change the primary key to a composite primary key (CODE_ID, DEPLOYMENT_ID) and remove the redundant UNIQUE (CODE_ID, DEPLOYMENT_ID) constraint so uniqueness is enforced per deployment; update the table definition to replace "CODE_ID VARCHAR(36) PRIMARY KEY" with CODE_ID VARCHAR(36) (no PK) and add "PRIMARY KEY (CODE_ID, DEPLOYMENT_ID)", ensuring DEPLOYMENT_ID column exists and is referenced in the composite key.
🧹 Nitpick comments (2)
backend/dbscripts/thunderdb/postgres.sql (2)
288-301: FLOW and FLOW_VERSION intentionally retain INT ID — worth a comment.These two tables are the only ones in the file that still use
INT GENERATED ALWAYS AS IDENTITYPKs. This is presumably becauseFLOW_VERSION.FLOW_INTERNAL_IDreferencesFLOW.ID, making the migration non-trivial. Consider adding a brief SQL comment (e.g.,-- Retains INT ID as PK since FLOW_VERSION references it) to document the rationale and prevent future confusion about why these tables differ from the rest.Also applies to: 309-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/postgres.sql` around lines 288 - 301, Add a brief SQL comment above the FLOW CREATE TABLE (and the FLOW_VERSION CREATE TABLE block) explaining why these two tables retain INT GENERATED ALWAYS AS IDENTITY primary keys; specifically note that FLOW_VERSION.FLOW_INTERNAL_ID references FLOW.ID so the INT PK was kept to avoid a non-trivial migration. Place a one-line comment like "-- Retains INT ID as PK since FLOW_VERSION.FLOW_INTERNAL_ID references FLOW.ID" immediately before the CREATE TABLE FLOW and similarly before the FLOW_VERSION definition to document the rationale.
142-152: AddUNIQUE (IDP_ID, DEPLOYMENT_ID),UNIQUE (NOTIFICATION_SENDER_ID, DEPLOYMENT_ID), andUNIQUE (ACTION_ID, DEPLOYMENT_ID)constraints for schema consistency.Tables like ROLE, THEME, LAYOUT, APPLICATION, RESOURCE_SERVER, RESOURCE, and FLOW all enforce a composite unique constraint pairing their primary key with DEPLOYMENT_ID. The IDP, NOTIFICATION_SENDER, and ACTION tables lack this constraint, breaking consistency. This is particularly important for ACTION, which already has composite foreign keys referencing RESOURCE_SERVER and RESOURCE with DEPLOYMENT_ID—if ACTION is ever referenced by another table via a composite FK, the lack of a matching UNIQUE constraint will cause schema inconsistency.
Suggested diffs
CREATE TABLE IDP ( DEPLOYMENT_ID VARCHAR(255) NOT NULL, IDP_ID VARCHAR(36) PRIMARY KEY, NAME VARCHAR(255) NOT NULL, DESCRIPTION VARCHAR(500), TYPE VARCHAR(20) NOT NULL, PROPERTIES JSONB, CREATED_AT TIMESTAMPTZ DEFAULT NOW(), - UPDATED_AT TIMESTAMPTZ DEFAULT NOW() + UPDATED_AT TIMESTAMPTZ DEFAULT NOW(), + UNIQUE (IDP_ID, DEPLOYMENT_ID) );CREATE TABLE NOTIFICATION_SENDER ( DEPLOYMENT_ID VARCHAR(255) NOT NULL, NAME VARCHAR(255) NOT NULL, SENDER_ID VARCHAR(36) PRIMARY KEY, DESCRIPTION VARCHAR(500), TYPE VARCHAR(20) NOT NULL, PROVIDER VARCHAR(20) NOT NULL, PROPERTIES JSONB, CREATED_AT TIMESTAMPTZ DEFAULT NOW(), - UPDATED_AT TIMESTAMPTZ DEFAULT NOW() + UPDATED_AT TIMESTAMPTZ DEFAULT NOW(), + UNIQUE (SENDER_ID, DEPLOYMENT_ID) );CREATE TABLE ACTION ( DEPLOYMENT_ID VARCHAR(255) NOT NULL, ACTION_ID VARCHAR(36) PRIMARY KEY, RESOURCE_SERVER_ID VARCHAR(36) NOT NULL, RESOURCE_ID VARCHAR(36), NAME VARCHAR(100) NOT NULL, HANDLE VARCHAR(100) NOT NULL, DESCRIPTION TEXT, PERMISSION VARCHAR(1000) NOT NULL, PROPERTIES JSONB, CREATED_AT TIMESTAMPTZ DEFAULT NOW(), UPDATED_AT TIMESTAMPTZ DEFAULT NOW(), - + UNIQUE (ACTION_ID, DEPLOYMENT_ID), FOREIGN KEY (RESOURCE_SERVER_ID, DEPLOYMENT_ID)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/postgres.sql` around lines 142 - 152, Add composite UNIQUE constraints pairing each table's primary id with DEPLOYMENT_ID to match the rest of the schema: update the CREATE TABLE for IDP to include UNIQUE (IDP_ID, DEPLOYMENT_ID), the CREATE TABLE for NOTIFICATION_SENDER to include UNIQUE (NOTIFICATION_SENDER_ID, DEPLOYMENT_ID), and the CREATE TABLE for ACTION to include UNIQUE (ACTION_ID, DEPLOYMENT_ID); if those tables are already created, add equivalent ALTER TABLE ... ADD CONSTRAINT statements instead so ACTION’s composite foreign key relationships remain valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/dbscripts/userdb/postgres.sql`:
- Line 4: Change the three tables that currently declare single-column PRIMARY
KEYs (OU with OU_ID, "USER" with USER_ID, and "GROUP" with GROUP_ID) to use
composite PRIMARY KEYs that include DEPLOYMENT_ID (i.e., PRIMARY KEY (OU_ID,
DEPLOYMENT_ID), PRIMARY KEY (USER_ID, DEPLOYMENT_ID), PRIMARY KEY (GROUP_ID,
DEPLOYMENT_ID)); remove the redundant UNIQUE constraints that duplicate those
composite keys; ensure any existing FOREIGN KEYs (e.g., in
GROUP_MEMBER_REFERENCE and USER_INDEXED_ATTRIBUTES which already reference
(GROUP_ID, DEPLOYMENT_ID) or (USER_ID, DEPLOYMENT_ID)) now match the new
composite PKs so referential integrity aligns with the multi-deployment design.
In `@backend/dbscripts/userdb/sqlite.sql`:
- Line 4: Replace the single-column primary key definitions for OU_ID, USER_ID,
and GROUP_ID with composite primary keys that include DEPLOYMENT_ID and mark the
ID columns NOT NULL: update the table definitions where OU_ID VARCHAR(36)
PRIMARY KEY, USER_ID VARCHAR(36) PRIMARY KEY, and GROUP_ID VARCHAR(36) PRIMARY
KEY appear so that each ID column is defined as VARCHAR(36) NOT NULL and add a
table-level PRIMARY KEY(DEPLOYMENT_ID, OU_ID) / PRIMARY KEY(DEPLOYMENT_ID,
USER_ID) / PRIMARY KEY(DEPLOYMENT_ID, GROUP_ID) respectively (ensure the
DEPLOYMENT_ID column exists and is NOT NULL in those tables).
---
Outside diff comments:
In `@backend/dbscripts/runtimedb/postgres.sql`:
- Around line 4-11: The current schema makes CODE_ID the primary key, making it
globally unique; change the primary key to a composite primary key (CODE_ID,
DEPLOYMENT_ID) and remove the redundant UNIQUE (CODE_ID, DEPLOYMENT_ID)
constraint so uniqueness is enforced per deployment; update the table definition
to replace "CODE_ID VARCHAR(36) PRIMARY KEY" with CODE_ID VARCHAR(36) (no PK)
and add "PRIMARY KEY (CODE_ID, DEPLOYMENT_ID)", ensuring DEPLOYMENT_ID column
exists and is referenced in the composite key.
---
Nitpick comments:
In `@backend/dbscripts/thunderdb/postgres.sql`:
- Around line 288-301: Add a brief SQL comment above the FLOW CREATE TABLE (and
the FLOW_VERSION CREATE TABLE block) explaining why these two tables retain INT
GENERATED ALWAYS AS IDENTITY primary keys; specifically note that
FLOW_VERSION.FLOW_INTERNAL_ID references FLOW.ID so the INT PK was kept to avoid
a non-trivial migration. Place a one-line comment like "-- Retains INT ID as PK
since FLOW_VERSION.FLOW_INTERNAL_ID references FLOW.ID" immediately before the
CREATE TABLE FLOW and similarly before the FLOW_VERSION definition to document
the rationale.
- Around line 142-152: Add composite UNIQUE constraints pairing each table's
primary id with DEPLOYMENT_ID to match the rest of the schema: update the CREATE
TABLE for IDP to include UNIQUE (IDP_ID, DEPLOYMENT_ID), the CREATE TABLE for
NOTIFICATION_SENDER to include UNIQUE (NOTIFICATION_SENDER_ID, DEPLOYMENT_ID),
and the CREATE TABLE for ACTION to include UNIQUE (ACTION_ID, DEPLOYMENT_ID); if
those tables are already created, add equivalent ALTER TABLE ... ADD CONSTRAINT
statements instead so ACTION’s composite foreign key relationships remain valid.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/dbscripts/runtimedb/postgres.sqlbackend/dbscripts/runtimedb/sqlite.sqlbackend/dbscripts/thunderdb/postgres.sqlbackend/dbscripts/thunderdb/sqlite.sqlbackend/dbscripts/userdb/postgres.sqlbackend/dbscripts/userdb/sqlite.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/dbscripts/runtimedb/sqlite.sql
KaveeshaPiumini
force-pushed
the
thunder
branch
2 times, most recently
from
428a723 to
2418ba2
Compare
@coderabbitai create an issue to make this change |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 47
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/dbscripts/thunderdb/postgres.sql (1)
251-272:⚠️ Potential issue | 🟡 MinorMissing
UNIQUE (ACTION_ID, DEPLOYMENT_ID)constraint on ACTION table.The
ACTIONtable usesACTION_IDas primary key but lacks aUNIQUE (ACTION_ID, DEPLOYMENT_ID)constraint. Other tables likeRESOURCE(line 225) andRESOURCE_SERVER(line 200) include this pattern to enable composite foreign key references in multi-deployment scenarios.Suggested fix
FOREIGN KEY (RESOURCE_ID, DEPLOYMENT_ID) REFERENCES RESOURCE(RESOURCE_ID, DEPLOYMENT_ID) ON DELETE RESTRICT - ON UPDATE CASCADE + ON UPDATE CASCADE, + UNIQUE (ACTION_ID, DEPLOYMENT_ID) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/postgres.sql` around lines 251 - 272, Add a composite unique constraint for ACTION_ID and DEPLOYMENT_ID on the ACTION table so it matches the pattern used by RESOURCE and RESOURCE_SERVER and allows other tables to reference ACTION via composite foreign keys; update the ACTION table DDL (table name ACTION) to include UNIQUE (ACTION_ID, DEPLOYMENT_ID) alongside the existing PRIMARY KEY and foreign keys.backend/dbscripts/thunderdb/sqlite.sql (1)
250-271:⚠️ Potential issue | 🟡 MinorMissing
UNIQUE (ACTION_ID, DEPLOYMENT_ID)constraint on ACTION table.Same issue as in the PostgreSQL schema:
ACTIONtable lacks the composite unique constraint that other entity tables have for multi-deployment FK support.Suggested fix
FOREIGN KEY (RESOURCE_ID, DEPLOYMENT_ID) REFERENCES RESOURCE(RESOURCE_ID, DEPLOYMENT_ID) ON DELETE RESTRICT - ON UPDATE CASCADE + ON UPDATE CASCADE, + UNIQUE (ACTION_ID, DEPLOYMENT_ID) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/thunderdb/sqlite.sql` around lines 250 - 271, Add the missing composite unique constraint on the ACTION table by adding UNIQUE(ACTION_ID, DEPLOYMENT_ID) to the CREATE TABLE ACTION statement so the ACTION_ID+DEPLOYMENT_ID tuple is enforced across deployments; update the CREATE TABLE ACTION block (which currently declares ACTION_ID VARCHAR(36) PRIMARY KEY and DEPLOYMENT_ID VARCHAR(255) NOT NULL) to include the UNIQUE(ACTION_ID, DEPLOYMENT_ID) constraint consistent with other entity tables and foreign-key usage.
🧹 Nitpick comments (1)
backend/dbscripts/runtimedb/postgres.sql (1)
2-12: Inconsistent PK pattern: consider composite PK(CODE_ID, DEPLOYMENT_ID)for multi-deployment support.
AUTHORIZATION_CODEuses a single-columnCODE_ID PRIMARY KEYwith a separateUNIQUE (CODE_ID, DEPLOYMENT_ID)constraint, while other tables in this same file (e.g.,AUTHORIZATION_REQUEST,FLOW_CONTEXT,WEBAUTHN_SESSION) use composite primary keys that includeDEPLOYMENT_ID. For consistency and to align with the multi-deployment architecture, consider making(CODE_ID, DEPLOYMENT_ID)the primary key.Suggested schema adjustment
CREATE TABLE AUTHORIZATION_CODE ( DEPLOYMENT_ID VARCHAR(255) NOT NULL, - CODE_ID VARCHAR(36) PRIMARY KEY, + CODE_ID VARCHAR(36) NOT NULL, AUTHORIZATION_CODE VARCHAR(500) NOT NULL, CLIENT_ID VARCHAR(255) NOT NULL, STATE VARCHAR(50) NOT NULL, AUTHZ_DATA JSONB NOT NULL, TIME_CREATED TIMESTAMP NOT NULL, EXPIRY_TIME TIMESTAMP NOT NULL, - UNIQUE (CODE_ID, DEPLOYMENT_ID) + PRIMARY KEY (CODE_ID, DEPLOYMENT_ID) );Based on learnings: "All tables must support multi-deployment scenarios by including
DEPLOYMENT_IDas part of the composite primary key."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dbscripts/runtimedb/postgres.sql` around lines 2 - 12, The AUTHORIZATION_CODE table currently declares CODE_ID as the single-column PRIMARY KEY and also a UNIQUE(CODE_ID, DEPLOYMENT_ID); change the table to use a composite primary key PRIMARY KEY (CODE_ID, DEPLOYMENT_ID) instead, remove the single-column primary key designation and the redundant UNIQUE constraint, and update any other schema objects (foreign keys, indexes, constraints) that reference CODE_ID alone to reference the composite key (CODE_ID, DEPLOYMENT_ID) so the table follows the multi-deployment PK pattern used by AUTHORIZATION_REQUEST, FLOW_CONTEXT and WEBAUTHN_SESSION.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/dbscripts/thunderdb/postgres.sql`:
- Around line 2-12: Add a unique constraint pairing SCHEMA_ID with DEPLOYMENT_ID
on the USER_SCHEMAS table: locate the CREATE TABLE USER_SCHEMAS definition and
add UNIQUE (SCHEMA_ID, DEPLOYMENT_ID) alongside the existing UNIQUE (NAME,
DEPLOYMENT_ID) so SCHEMA_ID can be used in composite foreign key relations;
ensure the constraint references the SCHEMA_ID column (currently declared as
PRIMARY KEY) and DEPLOYMENT_ID.
- Around line 143-152: The IDP and NOTIFICATION_SENDER tables are missing
composite UNIQUE constraints (UNIQUE (IDP_ID, DEPLOYMENT_ID) and UNIQUE
(SENDER_ID, DEPLOYMENT_ID)) which other tables use to support multi-deployment
composite foreign keys; update the CREATE TABLE for IDP (the definition that
includes DEPLOYMENT_ID and IDP_ID) to add UNIQUE (IDP_ID, DEPLOYMENT_ID) and
update the CREATE TABLE for NOTIFICATION_SENDER (the definition that includes
DEPLOYMENT_ID and SENDER_ID) to add UNIQUE (SENDER_ID, DEPLOYMENT_ID) so both
tables match the schema pattern used by ROLE/THEME/LAYOUT/APPLICATION and can be
referenced by composite foreign keys.
In `@backend/dbscripts/thunderdb/sqlite.sql`:
- Around line 2-12: USER_SCHEMAS is missing a composite unique constraint for
(SCHEMA_ID, DEPLOYMENT_ID); update the CREATE TABLE for USER_SCHEMAS to add
UNIQUE (SCHEMA_ID, DEPLOYMENT_ID) alongside the existing UNIQUE (NAME,
DEPLOYMENT_ID) so that SCHEMA_ID can be referenced in composite foreign keys —
modify the table definition containing columns SCHEMA_ID and DEPLOYMENT_ID to
include this additional UNIQUE constraint.
- Around line 141-151: Add missing unique multi-deployment constraints to the
SQLite schema by altering the table definitions for IDP and NOTIFICATION_SENDER:
add a UNIQUE constraint on (IDP_ID, DEPLOYMENT_ID) in the IDP table definition
and a UNIQUE constraint on (SENDER_ID, DEPLOYMENT_ID) in the NOTIFICATION_SENDER
table so their uniqueness semantics match the PostgreSQL schema and other
tables; update the CREATE TABLE statements that define IDP and
NOTIFICATION_SENDER to include these UNIQUE(...) clauses (refer to the IDP and
NOTIFICATION_SENDER table definitions to locate the changes).
---
Outside diff comments:
In `@backend/dbscripts/thunderdb/postgres.sql`:
- Around line 251-272: Add a composite unique constraint for ACTION_ID and
DEPLOYMENT_ID on the ACTION table so it matches the pattern used by RESOURCE and
RESOURCE_SERVER and allows other tables to reference ACTION via composite
foreign keys; update the ACTION table DDL (table name ACTION) to include UNIQUE
(ACTION_ID, DEPLOYMENT_ID) alongside the existing PRIMARY KEY and foreign keys.
In `@backend/dbscripts/thunderdb/sqlite.sql`:
- Around line 250-271: Add the missing composite unique constraint on the ACTION
table by adding UNIQUE(ACTION_ID, DEPLOYMENT_ID) to the CREATE TABLE ACTION
statement so the ACTION_ID+DEPLOYMENT_ID tuple is enforced across deployments;
update the CREATE TABLE ACTION block (which currently declares ACTION_ID
VARCHAR(36) PRIMARY KEY and DEPLOYMENT_ID VARCHAR(255) NOT NULL) to include the
UNIQUE(ACTION_ID, DEPLOYMENT_ID) constraint consistent with other entity tables
and foreign-key usage.
---
Nitpick comments:
In `@backend/dbscripts/runtimedb/postgres.sql`:
- Around line 2-12: The AUTHORIZATION_CODE table currently declares CODE_ID as
the single-column PRIMARY KEY and also a UNIQUE(CODE_ID, DEPLOYMENT_ID); change
the table to use a composite primary key PRIMARY KEY (CODE_ID, DEPLOYMENT_ID)
instead, remove the single-column primary key designation and the redundant
UNIQUE constraint, and update any other schema objects (foreign keys, indexes,
constraints) that reference CODE_ID alone to reference the composite key
(CODE_ID, DEPLOYMENT_ID) so the table follows the multi-deployment PK pattern
used by AUTHORIZATION_REQUEST, FLOW_CONTEXT and WEBAUTHN_SESSION.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/dbscripts/runtimedb/postgres.sqlbackend/dbscripts/runtimedb/sqlite.sqlbackend/dbscripts/thunderdb/postgres.sqlbackend/dbscripts/thunderdb/sqlite.sqlbackend/dbscripts/userdb/postgres.sqlbackend/dbscripts/userdb/sqlite.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/dbscripts/runtimedb/sqlite.sql
KaveeshaPiumini
force-pushed
the
thunder
branch
from
2418ba2 to
977c4a1
Compare
2 participants
Purpose
This pull request standardizes the primary key definitions across multiple database schema files for Postgres and SQLite. Specifically, it removes the use of auto-incremented integer
IDcolumns as primary keys and instead sets the relevant UUID-based columns (e.g.,*_ID,CODE_ID, etc.) as the primary keys for each table. This change improves consistency and aligns the schema with best practices for distributed systems where UUIDs are preferable for unique identification.Database schema changes:
Primary key standardization (Postgres & SQLite):
IDauto-incremented primary key columns from all major tables and set the appropriate UUID columns (e.g.,CODE_ID,SCHEMA_ID,ROLE_ID,THEME_ID,LAYOUT_ID,APP_ID,IDP_ID,SENDER_ID,CERT_ID,OU_ID,USER_ID,GROUP_ID) as the new primary keys in both runtime, thunder, and user database scripts. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23] [24]🔧 Summary of Breaking Changes
INT ID columns are removed from tables where it has not been used in any queries or for implementation.
💥 Impact
Direct database integrations, custom scripts, or downstream services that depend on the previous schema will require modification.
🔄 Migration Guide
Update database scripts to remove references to ID columns.
-->
Approach
Removal of auto-incrementing integer
IDprimary keys as we have UUIDv7 for existing UUID-based columns (such as*_IDcolumns). Since UUIDv7 is sequential, the additional INT ID column is not longer required and hence to be removed.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit