feature/Add DATABASE_SCHEMA support for PostgreSQL #5368
Conversation
azoz-w
changed the title
packages/server/src/DataSource.ts
Outdated
| migrationsRun: false, | ||
| entities: Object.values(entities), | ||
| migrations: postgresMigrations, | ||
| migrationsTableName: 'migrations', |
There was a problem hiding this comment.
is this necessary? can we use the default table name instead of hardcoding migrations as the table name
There was a problem hiding this comment.
True, im just completing the change as it seems a bit more larger. And ill refactor it better.
Thanks for the comment
packages/server/src/DataSource.ts
Outdated
| }) | ||
| break | ||
| case 'postgres': | ||
| const dbSchema = process.env.DATABASE_SCHEMA |
There was a problem hiding this comment.
you need to wrap these in curly brackets if using const
|
also if you can give a couple of screenshots to make sure this works, that'd be great |
WalkthroughThis pull request adds comprehensive PostgreSQL schema configuration support across the codebase. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI/Docker as CLI/Docker Config
participant Server as Server/DataSource
participant Utils as Schema Utilities
participant DB as PostgreSQL
User->>CLI/Docker: Set DATABASE_SCHEMA env var
CLI/Docker->>Server: Pass DATABASE_SCHEMA to process.env
Server->>Server: Read process.env.DATABASE_SCHEMA
Server->>Utils: Call getSchema(nodeData)
Utils->>Utils: Check nodeData.inputs?.schema
Utils->>Utils: Check process.env.POSTGRES_*_SCHEMA
Utils-->>Server: Return schema (or 'public' default)
Server->>Server: Sanitize and validate schema name
Server->>DB: Set connection search_path=${SCHEMA},public
Server->>DB: Execute queries with schema-qualified tables
DB-->>Server: Query results from correct schema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/components/nodes/vectorstores/Singlestore/Singlestore.ts (1)
175-175: Duplicate schema configuration concern.Same concern as line 134 - verify that SingleStore supports schema configuration and that this change is intentional for this PostgreSQL-focused PR.
🧹 Nitpick comments (5)
CONTRIBUTING.md (1)
143-143: Consider documenting the default schema value.While the documentation correctly describes the purpose, it might be helpful to specify that PostgreSQL defaults to the
publicschema when this variable is not set, as this is the standard PostgreSQL behavior.Consider this enhancement:
-| DATABASE_SCHEMA | Database schema (When DATABASE_TYPE is postgres) | String | | +| DATABASE_SCHEMA | Database schema (When DATABASE_TYPE is postgres) | String | `public` |packages/components/nodes/memory/AgentMemory/PostgresAgentMemory/pgSaver.ts (2)
8-8: Consider importing from a more appropriate location.The import is from
vectorstores/Postgres/utils, which creates a dependency between AgentMemory and vector stores. Given that a similargetSchemafunction exists inrecordmanager/PostgresRecordManager/utils.ts, consider whether that would be more appropriate, or if a shared utility location would be better for this common functionality.
21-21: Optimize schema retrieval and use strict equality.The current implementation calls
getSchema()twice, which is inefficient. Additionally, consider using strict equality (!==) instead of loose equality (!=).Apply this diff:
- this.tableName = getSchema() != 'public' ? `${getSchema()}.checkpoints` : 'checkpoints' + const schema = getSchema() + this.tableName = schema !== 'public' ? `${schema}.checkpoints` : 'checkpoints'packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
55-65: Consider extracting common sanitization logic.The sanitizeSchema method duplicates the logic from sanitizeTableName (lines 44-54). Consider extracting a common sanitization method to reduce duplication.
Example refactor:
private sanitizeIdentifier(identifier: string, type: string): string { // Trim and normalize case, turn whitespace into underscores identifier = identifier.trim().toLowerCase().replace(/\s+/g, '_') // Validate using a regex (alphanumeric and underscores only) if (!/^[a-zA-Z0-9_]+$/.test(identifier)) { throw new Error(`Invalid ${type}`) } return identifier } sanitizeTableName(tableName: string): string { return this.sanitizeIdentifier(tableName, 'table name') } sanitizeSchema(schema: string): string { return this.sanitizeIdentifier(schema, 'schema name') }packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts (1)
220-230: Consider refactoring to eliminate code duplication.The
sanitizeSchemaandsanitizeTableNamemethods contain nearly identical logic. Consider extracting a common helper method to reduce duplication.For example, you could create a generic sanitization method:
private sanitizeIdentifier(identifier: string, identifierType: string): string { // Trim and normalize case, turn whitespace into underscores identifier = identifier.trim().toLowerCase().replace(/\s+/g, '_') // Validate using a regex (alphanumeric and underscores only) if (!/^[a-zA-Z0-9_]+$/.test(identifier)) { throw new Error(`Invalid ${identifierType}`) } return identifier } sanitizeTableName(tableName: string): string { return this.sanitizeIdentifier(tableName, 'table name') } sanitizeSchema(schema: string): string { return this.sanitizeIdentifier(schema, 'schema name') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
CONTRIBUTING.md(1 hunks)docker/.env.example(1 hunks)docker/docker-compose-queue-prebuilt.yml(2 hunks)docker/docker-compose.yml(1 hunks)docker/worker/.env.example(1 hunks)docker/worker/docker-compose.yml(1 hunks)packages/components/nodes/memory/AgentMemory/PostgresAgentMemory/pgSaver.ts(3 hunks)packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts(12 hunks)packages/components/nodes/recordmanager/PostgresRecordManager/utils.ts(1 hunks)packages/components/nodes/vectorstores/Postgres/Postgres.ts(2 hunks)packages/components/nodes/vectorstores/Postgres/driver/Base.ts(3 hunks)packages/components/nodes/vectorstores/Postgres/driver/TypeORM.ts(2 hunks)packages/components/nodes/vectorstores/Postgres/utils.ts(1 hunks)packages/components/nodes/vectorstores/Singlestore/Singlestore.ts(2 hunks)packages/server/src/DataSource.ts(1 hunks)packages/server/src/commands/base.ts(2 hunks)packages/server/src/enterprise/middleware/passport/SessionPersistance.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
packages/components/nodes/recordmanager/PostgresRecordManager/utils.ts (3)
packages/components/nodes/vectorstores/Postgres/utils.ts (1)
getSchema(22-24)packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
getSchema(37-39)packages/components/src/utils.ts (1)
defaultChain(651-653)
packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
packages/components/nodes/vectorstores/Postgres/utils.ts (1)
getSchema(22-24)
packages/components/nodes/vectorstores/Postgres/Postgres.ts (2)
packages/components/nodes/vectorstores/Postgres/utils.ts (1)
getSchema(22-24)packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
getSchema(37-39)
packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts (2)
packages/components/nodes/recordmanager/PostgresRecordManager/utils.ts (2)
getSchema(10-12)getDatabase(7-9)packages/components/nodes/vectorstores/Postgres/driver/Base.ts (2)
getSchema(37-39)getDatabase(30-32)
packages/server/src/enterprise/middleware/passport/SessionPersistance.ts (1)
packages/server/src/DataSource.ts (1)
getDatabaseSSLFromEnv(114-124)
packages/server/src/DataSource.ts (2)
packages/server/src/database/entities/index.ts (1)
entities(29-59)packages/server/src/database/migrations/postgres/index.ts (1)
postgresMigrations(56-110)
packages/components/nodes/vectorstores/Postgres/utils.ts (3)
packages/components/nodes/recordmanager/PostgresRecordManager/utils.ts (1)
getSchema(10-12)packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
getSchema(37-39)packages/components/src/utils.ts (1)
defaultChain(651-653)
packages/components/nodes/memory/AgentMemory/PostgresAgentMemory/pgSaver.ts (2)
packages/components/nodes/vectorstores/Postgres/utils.ts (1)
getSchema(22-24)packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
getSchema(37-39)
🔇 Additional comments (25)
packages/components/nodes/vectorstores/Postgres/driver/TypeORM.ts (2)
16-16: LGTM - Schema retrieval.Correctly retrieves the schema configuration for use in connection options.
37-43: LGTM - Proper PostgreSQL schema configuration.The implementation correctly uses
search_pathto set the default schema for PostgreSQL connections, withpublicas a fallback. This is the standard approach for schema configuration in PostgreSQL via connection options.docker/docker-compose.yml (1)
16-16: LGTM - Proper environment variable propagation.The DATABASE_SCHEMA environment variable is correctly added and positioned alongside other database configuration variables.
docker/docker-compose-queue-prebuilt.yml (2)
31-31: LGTM - Consistent environment configuration.The DATABASE_SCHEMA variable is properly added to the flowise service, maintaining consistency with other database configuration variables.
176-176: LGTM - Worker service configuration.The DATABASE_SCHEMA variable is correctly propagated to the flowise-worker service, ensuring both main and worker services have consistent database schema configuration.
docker/worker/docker-compose.yml (1)
16-16: LGTM - Worker environment configuration.The DATABASE_SCHEMA environment variable is correctly added to the worker service configuration.
packages/server/src/commands/base.ts (2)
32-32: LGTM!The DATABASE_SCHEMA flag is correctly defined and placed logically within the database configuration flags.
163-163: LGTM!The environment variable mapping is correctly implemented and follows the established pattern for database configuration.
packages/components/nodes/vectorstores/Postgres/utils.ts (1)
22-24: LGTM!The getSchema function follows the same pattern as the other utility functions in this file, with proper fallback to 'public' as the default PostgreSQL schema.
packages/server/src/enterprise/middleware/passport/SessionPersistance.ts (1)
71-71: LGTM!The schema configuration is properly added to both the connection pool and session store options, with a sensible fallback to 'public'.
Also applies to: 77-77
packages/server/src/DataSource.ts (1)
73-73: LGTM!The schema configuration is properly set with a fallback to 'public', which is the standard PostgreSQL default schema.
packages/components/nodes/vectorstores/Postgres/Postgres.ts (1)
122-129: LGTM!The Schema input parameter is correctly configured following the same pattern as other optional configuration inputs like Table Name.
packages/components/nodes/memory/AgentMemory/PostgresAgentMemory/pgSaver.ts (1)
159-159: LGTM!Good cleanup - the trailing whitespace removal improves code formatting.
packages/components/nodes/recordmanager/PostgresRecordManager/utils.ts (1)
10-12: LGTM!The getSchema function follows the established pattern in this file and correctly uses the POSTGRES_RECORDMANAGER_SCHEMA environment variable specific to record managers.
packages/components/nodes/vectorstores/Postgres/driver/Base.ts (1)
37-39: LGTM!The getSchema method properly delegates to sanitizeSchema for validation, following the same pattern as getTableName.
packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts (10)
6-6: LGTM!The
getSchemaimport is properly added and will provide appropriate defaults through the fallback chain (input → environment variable → 'public').
77-84: LGTM!The Schema input field is properly configured with appropriate defaults and follows the same pattern as other optional database configuration fields.
171-172: LGTM!The schema is correctly added to the PostgreSQL connection options using the
getSchemahelper function.
198-206: LGTM!The schema property is properly declared, extracted from the connection options, and assigned to the instance.
439-443: LGTM!The
getFullTableNamemethod correctly builds a schema-qualified table name with proper sanitization. The fallback to an unqualified table name when no schema is provided is appropriate.
249-262: LGTM!All SQL statements in
createSchemaare properly updated to use the schema-qualified table name viagetFullTableName(). The sanitization ingetFullTableNameprovides protection against SQL injection.
314-332: LGTM!The
updatemethod correctly usesgetFullTableName()for the schema-qualified table name in the INSERT/UPSERT query.
351-356: LGTM!The
existsmethod correctly usesgetFullTableName()for the schema-qualified table name in the SELECT query.
372-374: LGTM!The
listKeysmethod correctly usesgetFullTableName()for the schema-qualified table name in the SELECT query.
426-429: LGTM!The
deleteKeysmethod correctly usesgetFullTableName()for the schema-qualified table name in the DELETE query.
| # DATABASE_PORT=5432 | ||
| # DATABASE_HOST="" | ||
| # DATABASE_NAME=flowise | ||
| # DATABASE_SCHEMA: default, |
There was a problem hiding this comment.
Fix the environment variable syntax.
The format uses : instead of = and includes a trailing comma. Environment variable declarations should use = without trailing punctuation to match the format of other variables in this file.
Apply this diff:
-# DATABASE_SCHEMA: default,
+# DATABASE_SCHEMA=default📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # DATABASE_SCHEMA: default, | |
| # DATABASE_SCHEMA=default |
🤖 Prompt for AI Agents
In docker/.env.example around line 14, the DATABASE_SCHEMA env var uses
incorrect syntax ("DATABASE_SCHEMA: default,"); change it to use '=' and remove
the trailing comma so it matches other entries (e.g., DATABASE_SCHEMA=default).
| # DATABASE_PORT=5432 | ||
| # DATABASE_HOST="" | ||
| # DATABASE_NAME=flowise | ||
| # # DATABASE_SCHEMA: default, |
There was a problem hiding this comment.
Fix the comment marker and environment variable syntax.
This line has multiple issues:
- Uses double
##instead of single#(inconsistent with other commented variables) - Uses
:instead of= - Includes a trailing comma
Apply this diff:
-# # DATABASE_SCHEMA: default,
+# DATABASE_SCHEMA=default📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # # DATABASE_SCHEMA: default, | |
| # DATABASE_SCHEMA=default |
🤖 Prompt for AI Agents
In docker/worker/.env.example around line 14, the commented environment variable
uses a double "##", a colon instead of "=", and has a trailing comma; replace
the leading "##" with a single "#", change the ":" to "=", and remove the
trailing comma so it matches other commented env vars (e.g., a single-line
commented key=value form).
|
|
||
| // Validate using a regex (alphanumeric and underscores only) | ||
| if (!/^[a-zA-Z0-9_]+$/.test(schema)) { | ||
| throw new Error('Invalid table name') |
There was a problem hiding this comment.
Fix the error message.
The error message says "Invalid table name" but should say "Invalid schema name" since this is the sanitizeSchema method.
Apply this diff:
- throw new Error('Invalid table name')
+ throw new Error('Invalid schema name')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new Error('Invalid table name') | |
| throw new Error('Invalid schema name') |
🤖 Prompt for AI Agents
In
packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts
around line 226 the thrown error message in sanitizeSchema is incorrect; replace
the message 'Invalid table name' with 'Invalid schema name' so the exception
accurately describes the validation failure.
packages/components/nodes/vectorstores/Singlestore/Singlestore.ts
Outdated
Show resolved
Hide resolved
| idleTimeoutMillis: 120000, | ||
| // set the search_path for the migrations and queries to the desired schema | ||
| options: `-c search_path=${process.env.DATABASE_SCHEMA},public` |
There was a problem hiding this comment.
Potential issue: Undefined schema in search_path.
If DATABASE_SCHEMA is not set, the template literal on line 82 will produce "undefined,public" instead of just "public". This could cause PostgreSQL to search for a schema named "undefined".
Apply this diff to fix the issue:
- options: `-c search_path=${process.env.DATABASE_SCHEMA},public`
+ options: `-c search_path=${process.env.DATABASE_SCHEMA || 'public'},public`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| idleTimeoutMillis: 120000, | |
| // set the search_path for the migrations and queries to the desired schema | |
| options: `-c search_path=${process.env.DATABASE_SCHEMA},public` | |
| idleTimeoutMillis: 120000, | |
| // set the search_path for the migrations and queries to the desired schema | |
| options: `-c search_path=${process.env.DATABASE_SCHEMA || 'public'},public` |
🤖 Prompt for AI Agents
In packages/server/src/DataSource.ts around lines 80 to 82, the options string
sets search_path using process.env.DATABASE_SCHEMA which can yield
"undefined,public" if the env var is missing; change the construction so it
falls back to only "public" or omits the custom schema when DATABASE_SCHEMA is
unset — e.g. compute a schema value = process.env.DATABASE_SCHEMA ?
`${process.env.DATABASE_SCHEMA},public` : 'public' and use that for options
(ensuring no literal "undefined" appears).
2 participants
This PR adds support for the DATABASE_SCHEMA environment variable. This allows users to specify a database schema rather than always defaulting to "public" when using a PostgreSQL database, which is necessary for multi-schema environments.
This also adds config inputs for the vector store and record manager to select the desired schema in PostgreSQL,