diff --git a/backend/test/ava-tests/non-saas-tests/non-saas-table-postgres-custom-types-e2e.test.ts b/backend/test/ava-tests/non-saas-tests/non-saas-table-postgres-custom-types-e2e.test.ts new file mode 100644 index 000000000..e63013c63 --- /dev/null +++ b/backend/test/ava-tests/non-saas-tests/non-saas-table-postgres-custom-types-e2e.test.ts @@ -0,0 +1,133 @@ +import { INestApplication, ValidationPipe } from '@nestjs/common'; +import { Test } from '@nestjs/testing'; +import test from 'ava'; +import { ValidationError } from 'class-validator'; +import cookieParser from 'cookie-parser'; +import { nanoid } from 'nanoid'; +import request from 'supertest'; +import { ApplicationModule } from '../../../src/app.module.js'; +import { WinstonLogger } from '../../../src/entities/logging/winston-logger.js'; +import { AllExceptionsFilter } from '../../../src/exceptions/all-exceptions.filter.js'; +import { ValidationException } from '../../../src/exceptions/custom-exceptions/validation-exception.js'; +import { Cacher } from '../../../src/helpers/cache/cacher.js'; +import { DatabaseModule } from '../../../src/shared/database/database.module.js'; +import { DatabaseService } from '../../../src/shared/database/database.service.js'; +import { MockFactory } from '../../mock.factory.js'; +import { getTestData } from '../../utils/get-test-data.js'; +import { getTestKnex } from '../../utils/get-test-knex.js'; +import { + createInitialTestUser, + registerUserAndReturnUserInfo, +} from '../../utils/register-user-and-return-user-info.js'; +import { setSaasEnvVariable } from '../../utils/set-saas-env-variable.js'; +import { TestUtils } from '../../utils/test.utils.js'; + +// Covers the Postgres structure read for tables whose columns use user-defined types (enums and +// composite types) against a real catalog. The DAO describes all of them with two batched catalog +// queries (see the unit suite for the query count); this suite checks what those queries return. + +const mockFactory = new MockFactory(); +let app: INestApplication; +let _testUtils: TestUtils; +let currentTest: string; + +const suffix = nanoid(6).replace(/[-_]/g, 'x').toLowerCase(); +const enumTypeName = `mood_${suffix}`; +const compositeTypeName = `address_${suffix}`; +const tableName = `people_${suffix}`; + +test.before(async () => { + setSaasEnvVariable(); + const moduleFixture = await Test.createTestingModule({ + imports: [ApplicationModule, DatabaseModule], + providers: [DatabaseService, TestUtils], + }).compile(); + app = moduleFixture.createNestApplication(); + _testUtils = moduleFixture.get(TestUtils); + + app.use(cookieParser()); + app.useGlobalFilters(new AllExceptionsFilter(app.get(WinstonLogger))); + app.useGlobalPipes( + new ValidationPipe({ + exceptionFactory(validationErrors: ValidationError[] = []) { + return new ValidationException(validationErrors); + }, + }), + ); + await app.init(); + await createInitialTestUser(app); + app.getHttpServer().listen(0); + + const knex = getTestKnex(getTestData(mockFactory).connectionToPostgres); + await knex.raw(`CREATE TYPE "${enumTypeName}" AS ENUM ('happy', 'sad', 'neutral')`); + await knex.raw(`CREATE TYPE "${compositeTypeName}" AS (street text, zip varchar(10))`); + await knex.raw( + `CREATE TABLE "${tableName}" ( + id serial PRIMARY KEY, + mood "${enumTypeName}" NOT NULL, + previous_mood "${enumTypeName}", + home "${compositeTypeName}", + note text + )`, + ); +}); + +test.after(async () => { + try { + const knex = getTestKnex(getTestData(mockFactory).connectionToPostgres); + await knex.raw(`DROP TABLE IF EXISTS "${tableName}"`); + await knex.raw(`DROP TYPE IF EXISTS "${compositeTypeName}"`); + await knex.raw(`DROP TYPE IF EXISTS "${enumTypeName}"`); + await Cacher.clearAllCache(); + await app.close(); + } catch (e) { + console.error('After tests error ' + e); + } +}); + +currentTest = 'GET /table/structure/:slug'; + +test.serial(`${currentTest} should describe enum and composite columns of a postgres table`, async (t) => { + const connectionToTestDB = getTestData(mockFactory).connectionToPostgres; + const firstUserToken = (await registerUserAndReturnUserInfo(app)).token; + + const createConnectionResponse = await request(app.getHttpServer()) + .post('/connection') + .send(connectionToTestDB) + .set('Cookie', firstUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(createConnectionResponse.status, 201); + const createConnectionRO = JSON.parse(createConnectionResponse.text); + + const getTableStructure = await request(app.getHttpServer()) + .get(`/table/structure/${createConnectionRO.id}?tableName=${tableName}`) + .set('Cookie', firstUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(getTableStructure.status, 200); + const getTableStructureRO = JSON.parse(getTableStructure.text); + + t.is(getTableStructureRO.structure.length, 5); + const byName = new Map(getTableStructureRO.structure.map((column) => [column.column_name, column])); + + t.is(byName.get('id').data_type, 'integer'); + t.is(byName.get('id').data_type_params, undefined); + + t.is(byName.get('mood').data_type, 'enum'); + t.deepEqual(byName.get('mood').data_type_params, ['happy', 'sad', 'neutral']); + t.is(byName.get('mood').allow_null, false); + + t.is(byName.get('previous_mood').data_type, 'enum'); + t.deepEqual(byName.get('previous_mood').data_type_params, ['happy', 'sad', 'neutral']); + t.is(byName.get('previous_mood').allow_null, true); + + t.is(byName.get('home').data_type, compositeTypeName); + t.deepEqual(byName.get('home').data_type_params, [ + { column_name: 'street', data_type: 'text' }, + { column_name: 'zip', data_type: 'character varying(10)' }, + ]); + + t.is(byName.get('note').data_type, 'text'); + t.is(byName.get('note').data_type_params, undefined); +}); diff --git a/backend/test/ava-tests/unit-tests/postgres-table-structure-custom-types.test.ts b/backend/test/ava-tests/unit-tests/postgres-table-structure-custom-types.test.ts new file mode 100644 index 000000000..e8b58fb08 --- /dev/null +++ b/backend/test/ava-tests/unit-tests/postgres-table-structure-custom-types.test.ts @@ -0,0 +1,209 @@ +import { DataAccessObjectPostgres } from '@rocketadmin/shared-code/dist/src/data-access-layer/data-access-objects/data-access-object-postgres.js'; +import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js'; +import test from 'ava'; +import { Knex } from 'knex'; + +// The Postgres DAO describes user-defined column types (enums, composite types) from the catalog +// while reading a table's structure. It used to run two catalog queries per USER-DEFINED column, +// sequentially and without deduplicating repeated types — the "N+1 query with repeating spans" +// Sentry reported on the pg_type/pg_class/pg_attribute SELECT. The structure must now be built +// from one composite-attributes query and one enum-labels query for the whole table. + +type RecordedQuery = { + from: string; + whereIn: Array<{ column: string; values: Array }>; +}; + +type RowResolver = (query: RecordedQuery) => Array>; + +function createFakeKnex(resolveRows: RowResolver): { knex: Knex; queries: Array } { + const queries: Array = []; + const createBuilder = (from: string) => { + const query: RecordedQuery = { from, whereIn: [] }; + queries.push(query); + const chain: any = { + select: () => chain, + from: (table: string) => { + query.from = table; + return chain; + }, + join: () => chain, + where: () => chain, + andWhere: () => chain, + whereIn: (column: string, values: Array) => { + query.whereIn.push({ column, values }); + return chain; + }, + orderBy: () => chain, + // biome-ignore lint/suspicious/noThenProperty: knex query builders are thenables, the DAO awaits them + then: (onFulfilled: (rows: unknown) => unknown, onRejected: (error: unknown) => unknown) => + Promise.resolve() + .then(() => resolveRows(query)) + .then(onFulfilled, onRejected), + }; + return chain; + }; + const knex: any = (table: string) => createBuilder(table); + knex.select = () => createBuilder(''); + knex.raw = (sql: string) => ({ sql }); + return { knex: knex as Knex, queries }; +} + +class StubbedPostgresDao extends DataAccessObjectPostgres { + private readonly fakeKnex: Knex; + + constructor(fakeKnex: Knex) { + super({ type: ConnectionTypesEnum.postgres, host: 'localhost', database: 'db' } as any); + this.fakeKnex = fakeKnex; + } + + protected override async configureKnex(): Promise> { + return this.fakeKnex; + } +} + +function columnRow( + column_name: string, + data_type: string, + udt_name: string, + overrides: Record = {}, +): Record { + return { + column_name, + column_default: null, + data_type, + udt_schema: 'public', + udt_name, + is_nullable: 'YES', + character_maximum_length: null, + is_identity: 'NO', + identity_generation: null, + ...overrides, + }; +} + +const TABLE_COLUMNS: Array> = [ + columnRow('id', 'integer', 'int4', { is_nullable: 'NO', identity_generation: 'BY DEFAULT' }), + columnRow('mood', 'USER-DEFINED', 'mood_t'), + columnRow('previous_mood', 'USER-DEFINED', 'mood_t'), + columnRow('home', 'USER-DEFINED', 'address_t'), + columnRow('location', 'USER-DEFINED', 'geometry'), + columnRow('note', 'text', 'text'), +]; + +const COMPOSITE_ATTRIBUTES: Array> = [ + { type_schema: 'public', type_name: 'address_t', attname: 'street', format_type: 'text' }, + { type_schema: 'public', type_name: 'address_t', attname: 'zip', format_type: 'character varying(10)' }, +]; + +const ENUM_LABELS: Array> = [ + { type_schema: 'public', type_name: 'mood_t', enumlabel: 'happy' }, + { type_schema: 'public', type_name: 'mood_t', enumlabel: 'sad' }, + { type_schema: 'public', type_name: 'mood_t', enumlabel: 'neutral' }, +]; + +function catalogResolver( + columns: Array>, + attributes: Array>, + enumLabels: Array>, +): RowResolver { + return (query) => { + switch (query.from) { + case 'information_schema.columns': + return columns; + case 'pg_type': + return attributes; + case 'pg_enum': + return enumLabels; + default: + throw new Error(`unexpected query source ${query.from}`); + } + }; +} + +test('describes every user-defined column with exactly one attributes query and one enum query', async (t) => { + const { knex, queries } = createFakeKnex(catalogResolver(TABLE_COLUMNS, COMPOSITE_ATTRIBUTES, ENUM_LABELS)); + const dao = new StubbedPostgresDao(knex); + + const structure = await dao.getTableStructureWithoutCache('people'); + + const catalogQueries = queries.filter((query) => query.from !== 'information_schema.columns'); + t.deepEqual( + catalogQueries.map((query) => query.from), + ['pg_type', 'pg_enum'], + 'four custom columns (three distinct types) must cost two catalog queries in total', + ); + for (const query of catalogQueries) { + t.is(query.whereIn.length, 1); + t.is(query.whereIn[0].column, 'pg_type.typname'); + t.deepEqual(query.whereIn[0].values, ['mood_t', 'address_t', 'geometry'], 'repeated types are looked up once'); + } + + const byName = new Map(structure.map((column) => [column.column_name, column])); + t.is(byName.get('mood').data_type, 'enum'); + t.deepEqual(byName.get('mood').data_type_params, ['happy', 'sad', 'neutral'] as any); + t.is(byName.get('previous_mood').data_type, 'enum'); + t.deepEqual(byName.get('previous_mood').data_type_params, ['happy', 'sad', 'neutral'] as any); + + t.is(byName.get('home').data_type, 'address_t'); + t.deepEqual(byName.get('home').data_type_params, [ + { column_name: 'street', data_type: 'text' }, + { column_name: 'zip', data_type: 'character varying(10)' }, + ] as any); + + // An extension base type (PostGIS geometry) is neither an enum nor a composite type: untouched. + t.is(byName.get('location').data_type, 'USER-DEFINED'); + t.is(byName.get('location').udt_name, 'geometry'); + t.is(byName.get('location').data_type_params, undefined); + + // The rest of the shape is unchanged by the batching. + t.is(byName.get('id').allow_null, false); + t.is(byName.get('id').extra, 'auto_increment'); + t.is(byName.get('note').allow_null, true); + t.is(byName.get('note').extra, undefined); + for (const column of structure) { + t.false('is_nullable' in column); + t.false('udt_schema' in column); + } +}); + +test('a table without user-defined columns issues no catalog queries', async (t) => { + const { knex, queries } = createFakeKnex( + catalogResolver([columnRow('id', 'integer', 'int4'), columnRow('note', 'text', 'text')], [], []), + ); + const dao = new StubbedPostgresDao(knex); + + const structure = await dao.getTableStructureWithoutCache('notes'); + + t.deepEqual( + queries.map((query) => query.from), + ['information_schema.columns'], + ); + t.is(structure.length, 2); +}); + +test('a same-named type in another schema does not leak its labels or attributes', async (t) => { + const columns = [ + columnRow('status', 'USER-DEFINED', 'status_t', { udt_schema: 'billing' }), + columnRow('shape', 'USER-DEFINED', 'shape_t', { udt_schema: 'billing' }), + ]; + const attributes = [ + { type_schema: 'public', type_name: 'shape_t', attname: 'sides', format_type: 'integer' }, + { type_schema: 'billing', type_name: 'shape_t', attname: 'kind', format_type: 'text' }, + ]; + const enumLabels = [ + { type_schema: 'public', type_name: 'status_t', enumlabel: 'public_only' }, + { type_schema: 'billing', type_name: 'status_t', enumlabel: 'open' }, + { type_schema: 'billing', type_name: 'status_t', enumlabel: 'closed' }, + ]; + const { knex } = createFakeKnex(catalogResolver(columns, attributes, enumLabels)); + const dao = new StubbedPostgresDao(knex); + + const structure = await dao.getTableStructureWithoutCache('invoices'); + + const byName = new Map(structure.map((column) => [column.column_name, column])); + t.is(byName.get('status').data_type, 'enum'); + t.deepEqual(byName.get('status').data_type_params, ['open', 'closed'] as any); + t.is(byName.get('shape').data_type, 'shape_t'); + t.deepEqual(byName.get('shape').data_type_params, [{ column_name: 'kind', data_type: 'text' }] as any); +}); diff --git a/shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts b/shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts index ba8b0cb69..cbc31c7b2 100644 --- a/shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts +++ b/shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts @@ -25,6 +25,15 @@ import { TestConnectionResultDS } from '../shared/data-structures/test-result-co import { ValidateTableSettingsDS } from '../shared/data-structures/validate-table-settings.ds.js'; import { BasicDataAccessObject } from './basic-data-access-object.js'; +type PostgresCustomTypeDescription = { + enumLabels: Array; + attributes: Array<{ column_name: string; data_type: string }>; +}; + +function customTypeKey(schema: string, name: string): string { + return `${schema}.${name}`; +} + export class DataAccessObjectPostgres extends BasicDataAccessObject implements IDataAccessObject { public async addRowInTable( tableName: string, @@ -379,6 +388,7 @@ export class DataAccessObjectPostgres extends BasicDataAccessObject implements I 'column_name', 'column_default', 'data_type', + 'udt_schema', 'udt_name', 'is_nullable', 'character_maximum_length', @@ -403,45 +413,36 @@ export class DataAccessObjectPostgres extends BasicDataAccessObject implements I }); if (customTypeIndexes.length > 0) { + // One catalog round trip per type kind for the whole table, not per column: a table with N + // user-defined columns used to issue 2N sequential catalog queries (composite attributes + + // enum labels, once per column, even for repeated types), which Sentry reported as an + // "N+1 query with repeating spans" on the pg_type/pg_class/pg_attribute SELECT. + const customTypes = await this.describeCustomTypes( + knex, + customTypeIndexes.map((index) => ({ + schema: result[index].udt_schema, + name: result[index].udt_name, + })), + ); for (const index of customTypeIndexes) { - const customTypeInTableName = result[index].udt_name; - const customTypeAttrsQueryResult = await knex.raw( - `select attname, format_type(atttypid, atttypmod) - from pg_type - join pg_class on pg_class.oid = pg_type.typrelid - join pg_attribute on pg_attribute.attrelid = pg_class.oid - where typname = ? - order by attnum`, - customTypeInTableName, - ); - const customTypeAttrs = customTypeAttrsQueryResult.rows; - const enumLabelQueryResult = await knex.raw( - `SELECT e.enumlabel - FROM pg_enum e - JOIN pg_type t ON e.enumtypid = t.oid - WHERE t.typname = ?`, - customTypeInTableName, - ); - let enumLabelRows = []; - if (enumLabelQueryResult?.rows && enumLabelQueryResult.rows.length > 0) { - enumLabelRows = enumLabelQueryResult.rows.map((el) => el.enumlabel); + const described = customTypes.get(customTypeKey(result[index].udt_schema, result[index].udt_name)); + if (!described) { + continue; } - if (enumLabelRows.length > 0) { + if (described.enumLabels.length > 0) { result[index].data_type = 'enum'; - result[index].data_type_params = enumLabelRows; + result[index].data_type_params = described.enumLabels; } - - if (customTypeAttrs && customTypeAttrs.length > 0) { - const customDataTypeRo = customTypeAttrs.map((attr) => ({ - column_name: attr.attname, - data_type: attr.format_type, - })); + if (described.attributes.length > 0) { result[index].data_type = result[index].udt_name; - result[index].data_type_params = customDataTypeRo; + result[index].data_type_params = described.attributes; } } } + for (const element of result) { + delete element.udt_schema; + } return result; } @@ -894,6 +895,58 @@ export class DataAccessObjectPostgres extends BasicDataAccessObject implements I return parseInt(fastCount, 10); } + /** + * Describes user-defined column types with two catalog queries for the whole set: composite + * attributes (pg_attribute of the type's row class) and enum labels. Types are matched by + * (schema, name) so a same-named type in another schema cannot leak its attributes or labels in. + */ + private async describeCustomTypes( + knex: Knex, + types: Array<{ schema: string; name: string }>, + ): Promise> { + const descriptions = new Map(); + for (const { schema, name } of types) { + descriptions.set(customTypeKey(schema, name), { enumLabels: [], attributes: [] }); + } + const typeNames = [...new Set(types.map(({ name }) => name))]; + + const [attributeRows, enumRows] = await Promise.all([ + knex + .select( + 'pg_namespace.nspname as type_schema', + 'pg_type.typname as type_name', + 'pg_attribute.attname as attname', + knex.raw('format_type(pg_attribute.atttypid, pg_attribute.atttypmod) as format_type'), + ) + .from('pg_type') + .join('pg_namespace', 'pg_namespace.oid', 'pg_type.typnamespace') + .join('pg_class', 'pg_class.oid', 'pg_type.typrelid') + .join('pg_attribute', 'pg_attribute.attrelid', 'pg_class.oid') + .whereIn('pg_type.typname', typeNames) + .andWhere('pg_attribute.attnum', '>', 0) + .andWhere('pg_attribute.attisdropped', false) + .orderBy(['pg_type.typname', 'pg_attribute.attnum']), + knex + .select('pg_namespace.nspname as type_schema', 'pg_type.typname as type_name', 'pg_enum.enumlabel as enumlabel') + .from('pg_enum') + .join('pg_type', 'pg_type.oid', 'pg_enum.enumtypid') + .join('pg_namespace', 'pg_namespace.oid', 'pg_type.typnamespace') + .whereIn('pg_type.typname', typeNames) + .orderBy(['pg_type.typname', 'pg_enum.enumsortorder']), + ]); + + for (const row of attributeRows) { + descriptions.get(customTypeKey(row.type_schema, row.type_name))?.attributes.push({ + column_name: row.attname, + data_type: row.format_type, + }); + } + for (const row of enumRows) { + descriptions.get(customTypeKey(row.type_schema, row.type_name))?.enumLabels.push(row.enumlabel); + } + return descriptions; + } + private attachSchemaNameToTableName(tableName: string): string { let fullTableName: string; if (this.connection.schema) {