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

evolve erase tries to delete citext type #253

Closed
peterhirn opened this issue Jan 7, 2022 · 19 comments · Fixed by #256 or #257
Closed

evolve erase tries to delete citext type #253

peterhirn opened this issue Jan 7, 2022 · 19 comments · Fixed by #256 or #257
Labels

Comments

@peterhirn
Copy link

peterhirn commented Jan 7, 2022

I'm creating a domain type based on citext

create extension if not exists citext;
create domain non_empty_citext as citext check (value ~ '\S');

Running evolve erase fails with

ERROR:  cannot drop type citext because extension citext requires it
HINT:  You can drop extension citext instead.
STATEMENT:  DROP TYPE IF EXISTS "public"."citext" CASCADE
ERROR:  current transaction is aborted, commands ignored until end of transaction block2022-01-07 07:47:50.102 UTC [97] STATEMENT:  SELECT pg_advisory_unlock(12345)

Not sure if I'm doing it wrong or this is a bug.

@lecaillon lecaillon added the bug label Jan 7, 2022
@lecaillon
Copy link
Owner

Thanks for the feedback. It seems to be a bug when trying to erase Postgresql extensions after creating a new domain. I'm gonna try to fix it as soon as possible.
Meanwhile, can you confirm you post all the scripts needed to easily reproduce the issue.
Can you tell me what version of Postgresql you use and everything that can help me ?
The fix will be available in evolve 3.1.0-alpha4

@peterhirn
Copy link
Author

I don't depend on this feature as I can just drop the database. No time pressure.

Versions

  • evolve (dotnet tool): 3.0.0
  • postgres (docker): postgres:14.1-alpine3.15

db/V1_0_0__works.sql

create extension if not exists pgcrypto;
create domain non_empty_text as text check (value ~ '\S');

vs.

db/V1_0_0__bug.sql

create extension if not exists citext;

test.ps1

Set-StrictMode -Version Latest
$connection="Server=127.0.0.1;Database=test;User Id=postgres;Password=password"

dotnet evolve migrate postgresql -l db -c "$connection"
if (-not $?) { throw "Migrate failed" }

dotnet evolve erase postgresql -c "$connection"
if (-not $?) { throw "Erase failed" }

@lecaillon
Copy link
Owner

I cannot reproduce your issue using evolve 3.1.0 alpha3. I created a new test Scenario9 that migrate then erase the schema. I use that SQL script:

create extension if not exists citext WITH SCHEMA ${schema};
create domain ${schema}.non_empty_citext as ${schema}.citext check (value ~ '\S');
create table ${schema}.test (
     did    integer PRIMARY KEY,
     name   ${schema}.non_empty_citext
);
insert into ${schema}.test values (1, 'PSG');

And everything works.
Note that I created the extension in a schema to not make my other tests fail. But even without it, it works.

@peterhirn
Copy link
Author

peterhirn commented Jan 8, 2022

Interesting. I upgraded to 3.1.0-alpha3 (dotnet tool) but erasing this migration still fails on my system:

create extension if not exists citext;

Servers logs DROP TYPE IF EXISTS "public"."citext" CASCADE

When using a schema I found a different issue. This works reliably (migrate then immediately erase):

create schema if not exists schema1;
create extension if not exists citext with schema schema1;

Running this works for the first execution, but on the second execution migrate fails:

create schema if not exists schema1;
create extension if not exists citext with schema schema1;
create domain schema1.non_empty_citext as schema1.citext check (value ~ '\S');

Error

Error executing script: V1_0_0__test.sql after 9 ms. 42710: type "non_empty_citext" already exists Sql query: --CREATE EXTENSION IF NOT EXISTS citext; -- --CREATE EXTENSION IF NOT EXISTS pgcrypto; -- --CREAT... 42710: type "non_empty_citext" already exists

@lecaillon
Copy link
Owner

lecaillon commented Jan 8, 2022

Strange. I don't understand why we do not have the same behavior. I use docker alpine latest image in my test. And the same version of Evolve. So ??? Maybe there is some other domain in other schema in your db that use "public"."citext" ?
Try to drop it yourself after Evolve fails to see if it is an Evolve issue or something else

@peterhirn
Copy link
Author

peterhirn commented Jan 8, 2022

Yes, very strange. I did drop and recreate the database after every failure, also using docker postgres:14.1-alpine3.15.

Passing the schema as argument eg. dotnet evolve migrate postgresql -s schema1 ... makes this work reliably:

create extension citext with schema schema1;
create domain schema1.non_empty_citext as schema1.citext check (value ~ '\S');

Give me a second, I'll try to reproduce in docker compose.

@peterhirn
Copy link
Author

Note for future readers: this is just a quick hack, please don't write dockerfiles like this (do proper cleanup, copy script files, don't embed connection strings, etc.).

Dockerfile

FROM debian:11.2-slim

ENV EVOLVE_VERSION="3.1.0-alpha3"
ENV CONNECTION="Server=db;Uid=postgres;Pwd=;Database=postgres"

RUN apt-get update \
    && apt-get install -y --no-install-recommends ca-certificates wget libicu67 \
    && wget -q https://github.com/lecaillon/Evolve/releases/download/${EVOLVE_VERSION}/evolve_${EVOLVE_VERSION}_Linux-64bit.tar.gz \
    && tar -xzvf evolve_${EVOLVE_VERSION}_Linux-64bit.tar.gz -C /usr/local/bin --strip-components 1

RUN mkdir db && echo "create extension if not exists citext;" > db/V1_0_0__test.sql

RUN echo "#!/bin/bash" > test.sh \
    && echo "evolve migrate postgresql -l db -c \"${CONNECTION}\"" >> test.sh \
    && echo "evolve erase postgresql -c \"${CONNECTION}\"" >> test.sh

RUN chmod +x test.sh \
    && cat test.sh

ENTRYPOINT [ "./test.sh" ]

docker-compose.yml

version: "3.9"
services:
  db:
    image: postgres:14.1-alpine3.15
    environment:
      POSTGRES_HOST_AUTH_METHOD: trust

  evolve:
    build: .
    links:
      - db

Run

docker compose rm -f
docker compose up --build

Output

evolve-bug-evolve-1  | Successfully applied migration V1_0_0__test.sql in 21 ms.
evolve-bug-evolve-1  | Database migrated to version 1.0.0. 1 migration(s) applied in 21 ms.
evolve-bug-evolve-1  | Evolve command 'migrate' successfully executed.
evolve-bug-db-1      | 2022-01-08 19:57:23.433 UTC [57] ERROR:  syntax error at or near "LIKE" at character 16
evolve-bug-db-1      | 2022-01-08 19:57:23.433 UTC [57] STATEMENT:  SHOW VARIABLES LIKE '%version%'
evolve-bug-evolve-1  | Evolve initialized.
evolve-bug-evolve-1  | Executing Erase...
evolve-bug-db-1      | 2022-01-08 19:57:23.525 UTC [57] ERROR:  cannot drop type citext because extension citext requires it
evolve-bug-db-1      | 2022-01-08 19:57:23.525 UTC [57] HINT:  You can drop extension citext instead.
evolve-bug-db-1      | 2022-01-08 19:57:23.525 UTC [57] STATEMENT:  DROP TYPE IF EXISTS "public"."citext" CASCADE
evolve-bug-db-1      | 2022-01-08 19:57:23.526 UTC [57] ERROR:  current transaction is aborted, commands ignored until end of transaction block
evolve-bug-db-1      | 2022-01-08 19:57:23.526 UTC [57] STATEMENT:  SELECT pg_advisory_unlock(12345)
evolve-bug-evolve-1  | 25P02: current transaction is aborted, commands ignored until end of transaction block Sql query: SELECT pg_advisory_unlock(12345) 25P02: current transaction is aborted, commands ignored until end of transaction block

@lecaillon
Copy link
Owner

Ok, i reproduce it when no schema is defined, so when the script executes in the schema public.
Btw like you dockerfile ^^

@lecaillon
Copy link
Owner

Ok, think I have a fix. I will release it later tomorrow ;)

@lecaillon
Copy link
Owner

Just published Evolve 3.1.0-alpha4
Happy testing :)

@peterhirn
Copy link
Author

peterhirn commented Jan 9, 2022

The docker compose sample produces the same error a different error after upgrading ENV EVOLVE_VERSION="3.1.0-alpha4" 😅

Edit: Extensions are not part of a schema, even if create with create extension citext with schema schema1; see https://www.postgresql.org/docs/current/sql-createextension.html

Remember that the extension itself is not considered to be within any schema: extensions have unqualified names that must be unique database-wide. But objects belonging to the extension can be within schemas.

Therefore dropping the extensions like evolve is now doing with drop extension if exists "schema1"."citext" cascade; is unfortunately invalid

ERROR:  syntax error at or near "."
LINE 1: drop extension if exists "schema1"."citext" cascade;
                                          ^

Edit2: Dropping extensions globally with cascade is (obviously) extremely dangerous.

@lecaillon
Copy link
Owner

lecaillon commented Jan 9, 2022

Ok, my bad. Was sure i had tested it. Unfortunately this cannot be part of my integration tests but I'm gonna try something else.
And normally no worry about your Edit2 because I only DROP extensions within the schema Evolve is responsible and for the role corresponding to the user that created it:

SELECT  e.extname 
FROM pg_extension  e 
LEFT JOIN pg_namespace n  ON n.oid = e.extnamespace 
LEFT JOIN pg_roles r ON  r.oid = e.extowner 
WHERE  n.nspname='{Name}' AND  r.rolname=current_user;

@lecaillon
Copy link
Owner

lecaillon commented Jan 9, 2022

Just published Evolve 3.1.0-alpha5. Try me ^^

@peterhirn
Copy link
Author

New error, stray double-quote 😁

evolve-bug-db-1      | 2022-01-10 16:57:34.649 UTC [57] ERROR:  unterminated quoted identifier at or near "" CASCADE" at character 32
evolve-bug-db-1      | 2022-01-10 16:57:34.649 UTC [57] STATEMENT:  DROP EXTENSION IF EXISTS citext" CASCADE

I only DROP extensions within the schema Evolve is responsible and for the role corresponding to the user that created it

Interesting, thanks for explaining. I didn't look into postgres users and roles at all yet. I'm still sceptical towards dropping extensions, maybe I'll play around with that later.

@lecaillon
Copy link
Owner

Tired of me ^^ Hopefully the 2nd times I added a new test scenario ... works well apparently :(

@lecaillon
Copy link
Owner

Ok understood why my test was successful... And it explains why deletion of a postgresql extension worked with a schema but doesn't work when associated to public.
All of this because during Erase(), Evolve DROP SCHEMA when it owns it... And keep the schema but DROP all objects one by one when it doesn't...

@lecaillon
Copy link
Owner

Evolve 3.1.0-alpha6 is out. What about now ? @peterhirn

@peterhirn
Copy link
Author

It works! 💪

@lecaillon
Copy link
Owner

lecaillon commented Jan 11, 2022

Thanks for your help on this one Peter. And thanks for the Dockerfile you post here and there. It is a very good starting point on how to use Evolve in a Docker pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants