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

False positive for prefer-robust-stmts being reported even inside a transaction #385

Closed
bmbferreira opened this issue Oct 1, 2024 · 2 comments · Fixed by #386
Closed
Labels

Comments

@bmbferreira
Copy link
Contributor

For the following script:

SELECT * FROM current_schema();

CREATE EXTENSION IF NOT EXISTS "uuid-ossp";

SELECT version();

SELECT * FROM "information_schema"."tables" WHERE "table_schema" = 'public' AND "table_name" = 'migrations';

SELECT * FROM "migrations" "migrations" ORDER BY "id" DESC;

START TRANSACTION;

ALTER TABLE "A" DROP CONSTRAINT "UQ_c4fb579a038211909ee524ccf29";

ALTER TABLE "B" DROP CONSTRAINT "UQ_791c01fe9438d66a94490d0da28";

ALTER TABLE "C" DROP CONSTRAINT "UQ_23fbf20e8ab4e806941359f4f79";

ALTER TABLE "D" DROP CONSTRAINT "UQ_468cad3743146a81c94b0b114ac";

COMMIT;

It is reporting:

extracted_statements.sql:12:2: warning: prefer-robust-stmts

  12 | ALTER TABLE "A" DROP CONSTRAINT "UQ_c4fb579a038211909ee524ccf29";

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

extracted_statements.sql:14:2: warning: prefer-robust-stmts

  14 | ALTER TABLE "B" DROP CONSTRAINT "UQ_791c01fe9438d66a94490d0da28";

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

extracted_statements.sql:16:2: warning: prefer-robust-stmts

  16 | ALTER TABLE "C" DROP CONSTRAINT "UQ_23fbf20e8ab4e806941359f4f79";

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

extracted_statements.sql:18:2: warning: prefer-robust-stmts

  18 | ALTER TABLE "D" DROP CONSTRAINT "UQ_468cad3743146a81c94b0b114ac";

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

As we can see, it is wrapped inside a transaction so I think nothing should be reported.

@sbdchd
Copy link
Owner

sbdchd commented Oct 1, 2024

ah seems we don't detect START TRANSACTION, as a workaround if you use BEGIN TRANSACTION it should work

@sbdchd sbdchd added the bug label Oct 1, 2024
@bmbferreira
Copy link
Contributor Author

unfortunately we're using TypeORM 😞 I think I don't have much control on how the transaction is started when executing the migrations.

I can try and open a PR to fix this 😄

@kodiakhq kodiakhq bot closed this as completed in #386 Oct 9, 2024
@kodiakhq kodiakhq bot closed this as completed in 12aa7b6 Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants