-
Notifications
You must be signed in to change notification settings - Fork 641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tables introduced via intra-vendor model routing feature to multi-dc scripts #12993
Conversation
WalkthroughThe changes update the database schema across multiple SQL dialects by adding new sequences, triggers, and tables while modifying existing columns. In Oracle, sequences and triggers are introduced to automatically assign values in the AM_API_PRIMARY_EP_MAPPING and AM_LLM_PROVIDER_MODEL tables. New tables—AM_API_ENDPOINTS, AM_API_PRIMARY_EP_MAPPING, and AM_LLM_PROVIDER_MODEL—are created in Oracle, PostgreSQL, and SQL Server, along with increasing the size of the PARAMETERS column in several policy mapping tables from VARCHAR(1024) to VARCHAR(2048). Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant T as Table
participant Tr as Trigger
participant S as Sequence
C->>T: Insert record with null ID
T->>Tr: Before Insert Trigger fires
Tr->>S: Request next sequence value
S-->>Tr: Return next value
Tr-->>T: Assign value and complete insert
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/sequences.sql (2)
216-219
: New Sequence Declarations for Intra-Vendor Routing Tables
The newly added sequences for the intra-vendor routing feature appear to follow the pattern of existing sequence definitions. However, note that the first new sequence uses the suffix_seq
(i.e. "AM_API_PRIMARY_EP_MAPPING_seq
") while the second one is declared as "AM_LLM_PROVIDER_MODEL_SEQ
" (in uppercase). For clarity and maintainability, it is recommended to standardize the naming convention across sequences.
1204-1221
: Trigger Additions for Primary Key Generation – Naming and Consistency Check
Two new triggers have been introduced to assign values automatically for the new tables. The trigger for the API primary endpoint mapping is named "AM_API_PRIMARY_EP_MAPPING_seq_tr
" and correctly uses a conditional check onNEW.MAPPING_ID
. In contrast, the trigger for the LLM provider model is named "AM_LLM_PROVIDER_MODEL_TRIGGER
" and does not include aWHEN (NEW.MODEL_ID IS NULL)
clause. Please verify if this difference is intentional. It might be beneficial to standardize naming and logic (for example, using_seq_tr
for both and including the conditional check for consistency) throughout the schema scripts.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/sequences_23c.sql (2)
215-218
: Consistent Sequence Definitions in Alternate SQL Script
The new sequences for the intra-vendor routing tables are added here as well. In this file bothAM_API_PRIMARY_EP_MAPPING_seq
andAM_LLM_PROVIDER_MODEL_seq
use a lowercase "seq" suffix consistently. This is good practice; however, please ensure that the naming conventions match across all related scripts (for instance, compare with the declarations in the main sequences.sql file).
1203-1220
: Trigger Definitions with Conditional Checks – Consistency Across Scripts
The triggers in this file, namelyAM_API_PRIMARY_EP_MAPPING_seq_tr
andAM_LLM_PROVIDER_MODEL_seq_tr
, both use a conditional check (WHEN (NEW.<column> IS NULL)
) before generating the next sequence value. This consistency is excellent and reduces the risk of overriding existing ID values. Just ensure that similar logic is applied across all SQL scripts to maintain uniformity.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/SQLServer/mssql/apimgt/tables.sql (1)
1747-1756
: Addition of AM_API_PRIMARY_EP_MAPPING table schema.
The table correctly introduces an identity columnMAPPING_ID
as the primary key and establishes a foreign key relationship with theAM_API
table viaAPI_UUID
. Consider whether a unique constraint on the combination ofAPI_UUID
andENDPOINT_UUID
should be added if duplicate mappings are not desired.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/tables.sql (1)
1499-1511
: New Table: AM_API_ENDPOINTS DefinitionThe new table for API endpoints is defined with a composite primary key (ENDPOINT_UUID, REVISION_UUID) and includes a unique constraint on (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION). The use of a default value ('Current API') for REVISION_UUID is clear. Please verify that the BLOB data type for ENDPOINT_CONFIG meets the system’s requirements and that appropriate indexing is in place if filtering by API_UUID or ENDPOINT_NAME is expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/sequences.sql
(2 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/sequences_23c.sql
(2 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/tables.sql
(5 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/tables_23c.sql
(5 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/Postgresql/apimgt/tables.sql
(5 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/SQLServer/mssql/apimgt/tables.sql
(5 hunks)
🔇 Additional comments (20)
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/tables_23c.sql (5)
1499-1511
: New Table Addition – AM_API_ENDPOINTS
The new tableAM_API_ENDPOINTS
is added to capture API endpoint details for the intra‑vendor model routing feature. The columns and constraints (including the unique constraint on(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)
and the primary key on(ENDPOINT_UUID, REVISION_UUID)
) appear to enforce uniqueness and referential integrity. Please verify that using a BLOB forENDPOINT_CONFIG
fits the intended design.
1512-1519
: New Table Addition – AM_API_PRIMARY_EP_MAPPING
This table defines the mapping for primary endpoints usingMAPPING_ID
,API_UUID
, andENDPOINT_UUID
. The foreign key reference onAPI_UUID
is appropriate. Ensure that the generation ofMAPPING_ID
(typically via a sequence/trigger mechanism, as handled in the related sequences files) is correctly set up in your overall deployment process.
2394-2402
: New Table Addition – AM_LLM_PROVIDER_MODEL
The new tableAM_LLM_PROVIDER_MODEL
is introduced to store models associated with LLM providers. The definition correctly enforces uniqueness on(MODEL_NAME, LLM_PROVIDER_UUID)
and sets a foreign key reference toAM_LLM_PROVIDER
. This adheres to the intended design for supporting new routing features.
2277-2288
: Schema Update – PARAMETERS Column in AM_API_OPERATION_POLICY_MAPPING
ThePARAMETERS
column has been altered toVARCHAR(2048)
to accommodate longer parameter strings. This change aligns with the PR objective for enhanced capacity. Please ensure that any necessary data migration or adaptation in dependent modules is addressed.
2289-2301
: Schema Update – PARAMETERS Column in AM_API_POLICY_MAPPING
Similarly, thePARAMETERS
column inAM_API_POLICY_MAPPING
is expanded toVARCHAR(2048)
, supporting the requirement for more extensive input. Verify that this change is propagated through all dependent services and that migration considerations (if data previously stored exceeds the old size) are handled.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/Postgresql/apimgt/tables.sql (6)
1863-1876
: New Table: AM_API_ENDPOINTS Implementation
This segment introduces the newAM_API_ENDPOINTS
table. The definition correctly specifies:
- A composite primary key using
ENDPOINT_UUID
andREVISION_UUID
- A unique constraint on the combination of
API_UUID
,ENDPOINT_NAME
,REVISION_UUID
, andORGANIZATION
- A foreign key constraint ensuring that
API_UUID
always refers to a valid value in theAM_API
table (with ON DELETE CASCADE).Everything appears consistent with the intra-vendor model routing feature requirements.
1877-1886
: New Table: AM_API_PRIMARY_EP_MAPPING Implementation
This hunk adds theAM_API_PRIMARY_EP_MAPPING
table along with its dedicated sequence. The table design includes:
- A sequence—
AM_API_PRIMARY_EP_MAPPING_SEQUENCE
—to generate uniqueMAPPING_ID
s- A foreign key on
API_UUID
referencingAM_API(API_UUID)
and an appropriate primary key onMAPPING_ID
The structure fulfills the design for maintaining primary endpoint mappings.
2925-2935
: New Table: AM_LLM_PROVIDER_MODEL Implementation
The changes correctly add theAM_LLM_PROVIDER_MODEL
table with:
- A new sequence (
AM_LLM_PROVIDER_MODEL_SEQ
) used to generateMODEL_ID
- A UNIQUE constraint to enforce that each combination of
MODEL_NAME
andLLM_PROVIDER_UUID
is unique- A foreign key linking
LLM_PROVIDER_UUID
to theAM_LLM_PROVIDER
tableThis table addition supports the new intra-vendor model routing feature as described.
2803-2804
: Schema Update: Increased PARAMETERS Column Size in AM_API_OPERATION_POLICY_MAPPING
ThePARAMETERS
column has been updated toVARCHAR(2048)
—an increase from the previous limit. This aligns with the PR’s objective to handle larger parameter values for policy mapping. Please verify that all application components using this column can accommodate the new size.
2819-2820
: Schema Update: Increased PARAMETERS Column Size in AM_API_POLICY_MAPPING
Here, thePARAMETERS
column in theAM_API_POLICY_MAPPING
table is resized toVARCHAR(2048)
. This change ensures consistency with the new capacity requirement across related policy mapping tables.
2843-2844
: Schema Update: Increased PARAMETERS Column Size in AM_GATEWAY_POLICY_MAPPING
The modification updates thePARAMETERS
column toVARCHAR(2048)
in theAM_GATEWAY_POLICY_MAPPING
table. This supports storing larger policy parameter values and aligns with the overall schema enhancement.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/SQLServer/mssql/apimgt/tables.sql (5)
1732-1746
: Addition of AM_API_ENDPOINTS table schema.
The creation of theAM_API_ENDPOINTS
table is well-defined. The composite primary key on(ENDPOINT_UUID, REVISION_UUID)
ensures uniqueness, while the unique constraint on(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)
enforces data integrity. Please verify that the default value'Current API'
for theREVISION_UUID
column is intentional and aligns with the application’s logic.
2681-2687
: Updated PARAMETERS column in AM_API_OPERATION_POLICY_MAPPING.
The addition ofPARAMETERS VARCHAR(2048) NOT NULL
meets the requirement to support larger data payloads. Ensure that any application logic or stored procedures interacting with this table are updated accordingly to accommodate the increased column size.
2696-2702
: Extended PARAMETERS column in AM_API_POLICY_MAPPING table.
Increasing thePARAMETERS
column toVARCHAR(2048)
is consistent with the schema upgrade for handling additional data. Please verify that this change does not adversely affect any dependent queries or processes that use this column.
2716-2721
: Update in AM_GATEWAY_POLICY_MAPPING: Expanded PARAMETERS column.
The modification to setPARAMETERS VARCHAR(2048) NOT NULL
aligns with the new routing feature requirements. It is important to confirm that any related application code or scripts that access this column have been updated to account for the extended length.
2794-2802
: Addition of AM_LLM_PROVIDER_MODEL table.
The newAM_LLM_PROVIDER_MODEL
table is appropriately defined with an identity columnMODEL_ID
as the primary key, a unique constraint on(MODEL_NAME, LLM_PROVIDER_UUID)
, and a foreign key reference toAM_LLM_PROVIDER(UUID)
. This design appears to properly enforce referential integrity and uniqueness; please verify that the chosen key structure meets the intra-vendor model routing feature’s design specifications.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/multi-dc/OGG/oracle/apimgt/tables.sql (4)
1512-1519
: New Table: AM_API_PRIMARY_EP_MAPPING DefinitionThe table AM_API_PRIMARY_EP_MAPPING is added to map APIs to endpoints. Its primary key is MAPPING_ID and it enforces referential integrity on API_UUID. As this table is central to the new intra-vendor model routing feature, please confirm whether additional unique constraints (e.g. combining API_UUID and ENDPOINT_UUID) are needed to prevent duplicate mappings, based on functional requirements.
2394-2402
: New Table: AM_LLM_PROVIDER_MODEL DefinitionThe AM_LLM_PROVIDER_MODEL table is introduced with MODEL_ID as the primary key and a unique constraint on the combination of MODEL_NAME and LLM_PROVIDER_UUID. The foreign key correctly references AM_LLM_PROVIDER(UUID). The design is consistent; however, please double-check that the chosen data types (e.g. VARCHAR(255) for MODEL_NAME) align with the rest of the schema and the expected input sizes.
2284-2284
: Modification: PARAMETERS Column in AM_API_OPERATION_POLICY_MAPPINGThe PARAMETERS column’s data type has been updated to VARCHAR(2048) to accommodate longer parameter configurations. Ensure that all application components consuming this column are updated to handle the increased capacity.
2297-2297
: Modification: PARAMETERS Column in AM_API_POLICY_MAPPINGSimilarly, the PARAMETERS column in AM_API_POLICY_MAPPING is now defined as VARCHAR(2048). This change aligns with the expanded capacity requirement. Please verify that any related application logic or stored procedures referencing this column are compatible with the new length.
Shall we make the sequence names and trigger names uppercase to be consistent with the rest of the script? |
Purpose
This PR contains the following table additions to multi-dc scripts:
Apart from the above,
Related Issue:
Summary by CodeRabbit