Skip to content
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

Ignore default property in FunctionParams while dropping a function #1198

Open
therealsujitk opened this issue Jun 13, 2024 · 1 comment
Open
Labels
c: bug Something isn't working good first issue Good for newcomers p: 1-normal Nothing urgent
Milestone

Comments

@therealsujitk
Copy link

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 therealsujitk added the s: pending triage Pending Triage label Jun 13, 2024
@therealsujitk
Copy link
Author

therealsujitk commented Jun 13, 2024

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

@Shinigami92 Shinigami92 added c: bug Something isn't working p: 1-normal Nothing urgent and removed s: pending triage Pending Triage labels Jun 13, 2024
@Shinigami92 Shinigami92 added this to the v7.x milestone Jun 13, 2024
@Shinigami92 Shinigami92 added the good first issue Good for newcomers label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working good first issue Good for newcomers p: 1-normal Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants