From bed04ec122234b4329a5e415bf3627c115b3f32e Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 5 Jan 2024 12:37:33 +0100 Subject: [PATCH] fix(Postgres Node): Stop marking autogenerated columns as required (#8230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Postgres columns can be - [generated as identity](https://www.postgresqltutorial.com/postgresql-tutorial/postgresql-identity-column/) - [generated by a custom expression](https://www.postgresql.org/docs/current/ddl-generated-columns.html) In these 2 cases, the column is not required when inserting a new row. This PR makes sure these types of column are not marked required in n8n. ### How to test 1. Create a Postgres table with all types of generated columns: for version >= 10 ```sql CREATE TABLE "public"."test_table" ( "id" int8 NOT NULL DEFAULT nextval('test_table_id_seq'::regclass), "identity_id" bigint GENERATED ALWAYS AS IDENTITY, "id_plus" numeric GENERATED ALWAYS AS (id + 5) STORED, "title" varchar NOT NULL, "created_at" timestamp DEFAULT now(), PRIMARY KEY ("id") ) ``` Before 10 you have to use serial or bigserial types: ```sql CREATE TABLE distributors ( did serial not null primary key, name varchar(40) NOT NULL CHECK (name <> '') ); ``` 2. Add a postgres node to canvas and try to insert data without the generated columns 3. Should successfully insert More info in Linear/Github issue ⬇️ ## Related tickets and issues - fixes #7084 - https://linear.app/n8n/issue/NODE-816/rmc-not-all-id-fields-should-be-required - https://linear.app/n8n/issue/NODE-681/postgres-cant-map-automatically-if-database-requires-a-field ## Review / Merge checklist - [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. > A feature is not complete without tests. --------- Co-authored-by: Michael Kret --- .../nodes/Postgres/v2/helpers/interfaces.ts | 2 ++ .../nodes/Postgres/v2/helpers/utils.ts | 34 ++++++++++++++++++- .../Postgres/v2/methods/resourceMapping.ts | 8 +++-- packages/workflow/src/NodeHelpers.ts | 5 +++ 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/nodes-base/nodes/Postgres/v2/helpers/interfaces.ts b/packages/nodes-base/nodes/Postgres/v2/helpers/interfaces.ts index 13f731a02a..57d5c6b22a 100644 --- a/packages/nodes-base/nodes/Postgres/v2/helpers/interfaces.ts +++ b/packages/nodes-base/nodes/Postgres/v2/helpers/interfaces.ts @@ -17,6 +17,8 @@ export type ColumnInfo = { is_nullable: string; udt_name?: string; column_default?: string; + is_generated?: 'ALWAYS' | 'NEVER'; + identity_generation?: 'ALWAYS' | 'NEVER'; }; export type EnumInfo = { typname: string; diff --git a/packages/nodes-base/nodes/Postgres/v2/helpers/utils.ts b/packages/nodes-base/nodes/Postgres/v2/helpers/utils.ts index c9ec84ffdd..7367a62af4 100644 --- a/packages/nodes-base/nodes/Postgres/v2/helpers/utils.ts +++ b/packages/nodes-base/nodes/Postgres/v2/helpers/utils.ts @@ -376,13 +376,45 @@ export function prepareItem(values: IDataObject[]) { return item; } +export async function columnFeatureSupport( + db: PgpDatabase, +): Promise<{ identity_generation: boolean; is_generated: boolean }> { + const result = await db.any( + `SELECT EXISTS ( + SELECT 1 FROM information_schema.columns WHERE table_name = 'columns' AND table_schema = 'information_schema' AND column_name = 'is_generated' + ) as is_generated, + EXISTS ( + SELECT 1 FROM information_schema.columns WHERE table_name = 'columns' AND table_schema = 'information_schema' AND column_name = 'identity_generation' + ) as identity_generation;`, + ); + + return result[0]; +} + export async function getTableSchema( db: PgpDatabase, schema: string, table: string, + options?: { getColumnsForResourceMapper?: boolean }, ): Promise { + const select = ['column_name', 'data_type', 'is_nullable', 'udt_name', 'column_default']; + + if (options?.getColumnsForResourceMapper) { + // Check if columns exist before querying (identity_generation was added in v10, is_generated in v12) + const supported = await columnFeatureSupport(db); + + if (supported.identity_generation) { + select.push('identity_generation'); + } + + if (supported.is_generated) { + select.push('is_generated'); + } + } + + const selectString = select.join(', '); const columns = await db.any( - 'SELECT column_name, data_type, is_nullable, udt_name, column_default FROM information_schema.columns WHERE table_schema = $1 AND table_name = $2', + `SELECT ${selectString} FROM information_schema.columns WHERE table_schema = $1 AND table_name = $2`, [schema, table], ); diff --git a/packages/nodes-base/nodes/Postgres/v2/methods/resourceMapping.ts b/packages/nodes-base/nodes/Postgres/v2/methods/resourceMapping.ts index 08959acc6b..cad0923b05 100644 --- a/packages/nodes-base/nodes/Postgres/v2/methods/resourceMapping.ts +++ b/packages/nodes-base/nodes/Postgres/v2/methods/resourceMapping.ts @@ -62,7 +62,7 @@ export async function getMappingColumns( }) as string; try { - const columns = await getTableSchema(db, schema, table); + const columns = await getTableSchema(db, schema, table, { getColumnsForResourceMapper: true }); const unique = operation === 'upsert' ? await uniqueColumns(db, table, schema) : []; const enumInfo = await getEnums(db); const fields = await Promise.all( @@ -72,11 +72,13 @@ export async function getMappingColumns( const type = mapPostgresType(col.data_type); const options = type === 'options' ? getEnumValues(enumInfo, col.udt_name as string) : undefined; - const isAutoIncrement = col.column_default?.startsWith('nextval'); + const hasDefault = Boolean(col.column_default); + const isGenerated = col.is_generated === 'ALWAYS' || col.identity_generation === 'ALWAYS'; + const nullable = col.is_nullable === 'YES'; return { id: col.column_name, displayName: col.column_name, - required: col.is_nullable !== 'YES' && !isAutoIncrement, + required: !nullable && !hasDefault && !isGenerated, defaultMatch: (col.column_name === 'id' && canBeUsedToMatch) || false, display: true, type, diff --git a/packages/workflow/src/NodeHelpers.ts b/packages/workflow/src/NodeHelpers.ts index 50474011cf..7a840b05fd 100644 --- a/packages/workflow/src/NodeHelpers.ts +++ b/packages/workflow/src/NodeHelpers.ts @@ -1230,6 +1230,11 @@ export const validateResourceMapperParameter = ( value: ResourceMapperValue, skipRequiredCheck = false, ): Record => { + // No issues to raise in automatic mapping mode, no user input to validate + if (value.mappingMode === 'autoMapInputData') { + return {}; + } + const issues: Record = {}; let fieldWordSingular = nodeProperties.typeOptions?.resourceMapper?.fieldWords?.singular || 'Field';