Extended posgres Drop() with type dropping #627
Conversation
Pull Request Test Coverage Report for Build 1459982313
💛 - Coveralls |
8a96bb7 to
9159f93
Compare
| switch element { | ||
| case tableSQL: | ||
| query = `SELECT table_name FROM information_schema.tables WHERE table_schema=(SELECT current_schema()) AND table_type='BASE TABLE'` | ||
| case typeSQL: |
There was a problem hiding this comment.
Add tests for this. e.g. that this will only drop unused types in the current schema
There was a problem hiding this comment.
This function does not check for unused. Drop() however does what you expect it would. First it drops all the tables. Than it drops all the custom types. Because the tables are dropped first all the custom types are unused.
The expected result is now tested.
| SELECT t.typname as type | ||
| FROM pg_type t | ||
| LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace | ||
| WHERE (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid)) |
There was a problem hiding this comment.
Can you add comments for this query? It's not immediately clear to me what the filtering conditions are doing due to my lack of familiarity with the postgres system catalogs.
Docs:
- https://www.postgresql.org/docs/current/catalog-pg-type.html
- Do
typelmandtyparraytrack references to the type?
- Do
- https://www.postgresql.org/docs/current/catalog-pg-namespace.html
- https://www.postgresql.org/docs/current/catalog-pg-class.html
Also, shouldn't
WHERE (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))be
WHERE (t.typrelid = 0 OR (SELECT 1 FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid AND c.relkind = 'c'))There was a problem hiding this comment.
Thanks for the documentation I had some time to look into it a bit. I'm very new to postgresql so a few weeks before this PR I did not even know arrays where possible. I took this query from stackoverflow and adjusted it for the current schema and tested it. The source of the query I modified is actually psql \dT.
Also, shouldn't
WHERE (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))
be
WHERE (t.typrelid = 0 OR (SELECT 1 FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid AND c.relkind = 'c'))
No the query is correct. Your suggestion will result in a syntax error (subquery needs to return a boolean). I don´t fully understand why this is needed because I'm unfamiliar with the administration of composite types. What this WHERE does is include all composite and non-composite types. Types can apparently also have a typerel of a table or index etc. In the source of psql is is described as
do not include complex types (typrelid!=0) unless they are standalone composite types.
I will do my best to formulate a comprehensive comment.
|
This will fix #193 |
|
Ah missed that issue. Created a duplicate. Yes sometimes we don't care or need to care about the Down. Down is specifically only for things created by Up. |
debb120 to
bf9df6c
Compare
bf9df6c to
19eb878
Compare
|
@dhui Can you please have another look? I'm staring to get out of context again. |
|
Hi @dhui could you please continue the review? |
|
just got bit by this. would be great if this could be reviewed and merged soon! |
|
Same here |
|
Also stumbled upon this issue caused by #193 and lost some minutes trying to figure out what was going on, this getting reviewed and merged would be very appreciated! |
|
merge? when? |
Most likely never. I've tried it. Without commitment from maintainers I'm not rebasing. |
Closes #626
Fixes that
Migrate.Drop()is not clearing the database properly in both thepostgresandpgxdatabase driver.