salsita / node-pg-migrate

Node.js database migration management for PostgreSQL
https://salsita.github.io/node-pg-migrate
MIT License
1.27k stars 174 forks source link

Ignore `default` property in `FunctionParams` while dropping a function #1198

Open therealsujitk opened 2 months ago

therealsujitk commented 2 months ago

Description

Currently when you drop a function, it requires you to pass the parameters for the function, and since I don't want to write these parameters twice I create a common variable and use it in both the up and the down migrations. The issue is if a parameter has a default value set, dropping the function throws an error.

const public_my_func = { schema: 'public', name: 'my_func' };
const public_my_func_params = [
  { name: '_param', type: PgType.TEXT, default: PgLiteral.create("''") }
];

pgm.dropFunction(public_my_func, public_my_func_params);
DROP FUNCTION "public"."my_func"("_param" text DEFAULT '');
> Rolling back attempted migration ...
error: syntax error at or near "DEFAULT"
    at /.../node_modules/pg/lib/client.js:526:17
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Object.query (/.../node_modules/node-pg-migrate/dist/db.js:70:14)
    at async runner (/.../node_modules/node-pg-migrate/dist/runner.js:260:9) {
  length: 96,
  severity: 'ERROR',
  code: '42601',
  detail: undefined,
  hint: undefined,
  position: '85',
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'scan.l',
  line: '1188',
  routine: 'scanner_yyerror'
}

Suggested solution

Ignore the default property in the dropFunction() method while creating the query.

Alternative

A workaround for this would be to recreate the params array for the delete function without the default property, or to create a helper method that deletes this property and returns a new array that can be used to drop the function.

Additional context

No response

therealsujitk commented 2 months ago

On a separate note I noticed a bug, if you specify default as '' (an empty string), it simply ignores it. I'm guessing this is because to check if a default value is set we use if (param.default) { ... } or something similar, but an empty string returns false as well and so it gets ignored.

Edit: Here's the issue --> https://github.com/salsita/node-pg-migrate/blob/main/src/utils/formatParams.ts#L30