perseas / Pyrseas

Provides utilities for Postgres database schema versioning.
https://perseas.github.io/
BSD 3-Clause "New" or "Revised" License
395 stars 67 forks source link

A table called "group" causes a crash #196

Closed xclayl closed 6 years ago

xclayl commented 6 years ago

I think it's related to foreign keys. My guess is the FKs don't have the double quotes around "group" like it does below. I'm using version 0.8. It seems fine in version 0.7.x.

error>> Traceback (most recent call last): error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\runpy.py", line 193, in _run_module_as_main error>> "main", mod_spec) error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\runpy.py", line 85, in _run_code error>> exec(code, run_globals) error>> File "C:\Users\clay\AppData\Local\Programs\Python\Python36-32\Scripts\yamltodb.exe__main__.py", line 9, in error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\site-packages\pyrseas\yamltodb.py", line 50, in main error>> stmts = db.diff_map(inmap) error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\site-packages\pyrseas\database.py", line 510, in diff_map error>> self.from_catalog() error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\site-packages\pyrseas\database.py", line 352, in from_catalog error>> self._link_refs(self.db) error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\site-packages\pyrseas\database.py", line 223, in _link_refs error>> db.triggers) error>> File "c:\users\clay\appdata\local\programs\python\python36-32\lib\site-packages\pyrseas\dbobject\table.py", line 956, in link_refs error>> assert self[(sch, tbl)] error>> KeyError: ('app', '"group"') error>> out>>

jmafc commented 6 years ago

I'm afraid I need some test data or a reproducible test case. FWIW, I created two tables, one called group and the second other, each having foreign keys to each others respective primary key. I ran dbtoyaml first since the _from_catalog that appears in the traceback is called from both utilities. It produced YAML output without errors. Passing that output to yamltodb also didn't result in errors, both against the same and a different database.

xclayl commented 6 years ago

Thanks for looking into this. I was able to reproduce it with a simple scenario -- it has something to do with reading the table name from the server, rather than the yaml file. So first time it works. Second time it runs it crashes.

table.group.yaml:

table group:
  columns:
  - group_id:           {not_null: true, type: bigint} 
  primary_key:
    pk_group: { columns: [ group_id ] }
  owner: postgres

table.group_member.yaml:

table group_member:
  columns:
  - group_member_id:    {not_null: true, type: bigint}
  - group_id:           {not_null: true, type: bigint}
  foreign_keys:
    group_member_group_id_fkey:             { columns: [ group_id ], references: {columns: [ group_id ],  schema: app, table: group }}
  owner: postgres

database.mydb.yaml:

schema app:
  schema: schema.app.yaml
  table group: schema.app\table.group.yaml
  table group_member: schema.app\table.group_member.yaml
schema public:

yamltodb -U postgres -H localhost -u --multiple-files -W pytest

jmafc commented 6 years ago

I'm afraid I'm still unable to reproduce. What I did is concatenate your two files, added a line for "schema app:" and properly indented the table specs. I then ran yamltodb twice against a 9.6 database using Python 3.6. The first time it created the schema and the tables, the second time there was no output or error. I also tried it with --multiple-files and got the same response, and also tested with Python 2.7 just for completeness.

xclayl commented 6 years ago

I'm using Postgres 9.6 as well.

On a whim, I executed this on the database, which comes from the yamltodb source code (line 408 in constraint.py).

SELECT conname AS name, nspname AS schema,
                   conrelid::regclass AS table, conkey AS columns,
                   condeferrable AS deferrable, condeferred AS deferred,
                   confrelid::regclass AS ref_table, confkey AS ref_cols,
                   confupdtype AS on_update, confdeltype AS on_delete,
                   confmatchtype AS match, amname AS access_method,
                   spcname AS tablespace, c.oid,
                   indisclustered AS cluster, coninhcount > 0 AS inherited,
                   obj_description(c.oid, 'pg_constraint') AS description
            FROM pg_constraint c
                 JOIN pg_namespace ON (connamespace = pg_namespace.oid)
                 JOIN pg_index i ON (indexrelid = conindid)
                 JOIN pg_class cl ON (indexrelid = cl.oid)
                 JOIN pg_am on (relam = pg_am.oid)
                 LEFT JOIN pg_tablespace t ON (cl.reltablespace = t.oid)
            WHERE nspname != 'pg_catalog' AND nspname != 'information_schema'
                  AND nspname NOT LIKE 'pg_temp\_%'
                  AND nspname NOT LIKE 'pg_toast_temp\_%'
              AND contype = 'f'
              AND contypid NOT IN (SELECT objid FROM pg_depend
                                   WHERE deptype = 'e'
                                     AND classid = 'pg_type'::regclass)
            ORDER BY schema, "table", name

The ref_table column contained app."group". When I rename the table to "groups", the double quotes aren't in the ref_table column. Does this help at all?

xclayl commented 6 years ago

I'm using "PostgreSQL 9.6.1, compiled by Visual C++ build 1800, 64-bit". I'll upgrade to the latest (9.6.10) to see how it goes

xclayl commented 6 years ago

The query above returns group in double quotes on 9.6.10 as well. Attached are the yaml files I'm using.

pytest.zip

jmafc commented 6 years ago

The double quotes in app."group" are normal Postgres behavior. However, that is "cleaned up" by the fix to #186, i.e., change 754abb8. That explains why I'm not seeing the problem. I'm testing with the HEAD of r0.8 which already has that fix.

jmafc commented 6 years ago

So you can either apply the change to your code or wait for a maintenance release.

xclayl commented 6 years ago

Glad it's fixed :) Thanks for looking into it.