Pyrseas
Pyrseas copied to clipboard
Table named with PG reserved word and constraint causes error in dbtoyaml
Test case:
CREATE TABLE "order" (
c1 integer UNIQUE,
c2 text);
Running dbtoyaml
causes error in https://github.com/perseas/Pyrseas/blob/fdbb76d05c87dcd92941f3b48309f7dfe4ccf45f/pyrseas/dbobject/table.py#L927 as follows:
KeyError: ('public', '"order"')
.
The same problem occurs if UNIQUE
is replaced by PRIMARY KEY
.
https://github.com/perseas/Pyrseas/blob/171d703ce52bd61055058ed48dca3c62654b5f17/pyrseas/dbobject/init.py#L37-L39
The identifier quoting is currently only done for what Pyrseas thinks are reserved words. Unfortunately, that criteria misses some other reserved keywords:
SELECT word FROM pg_get_keywords() WHERE catdesc like 'reserved%' and catcode != 'R';
word
----------------
authorization
binary
collation
concurrently
cross
current_schema
freeze
full
ilike
inner
is
isnull
join
left
like
natural
notnull
outer
overlaps
right
similar
tablesample
verbose
(23 rows)
In my experience, query builders (as found in things like django, Ruby on Rails, etc.) generally unconditionally quote all identifiers.
https://github.com/perseas/Pyrseas/blob/171d703ce52bd61055058ed48dca3c62654b5f17/pyrseas/dbobject/init.py#L58
I can certainly see how it could be a breaking change to switch to it, but that aside it seems like a much safer approach. Is there a reason this is only done conditionally in Pyrseas that I'm missing?
Even if the quoting is left as conditional, the reserved words list is definitely incomplete.
It's been a while, but I just tested the example that I created against both PG 9.6.16 and 11.6-1 database, with both the Pyrseas stable release and master, and using both Python 2.7.16 and 3.7.3, and dbtoyaml
does not fail. Do you have an example of where it's happening to you?
dbtoyaml
is not a query builder: its output is YAML. The error that was happening before was purely internal. IIRC, that list of RESERVED_WORDS
is only used by yamltodb
.
Sorry, I misread the issue title. I am indeed talking about yamltodb
.
OK, so if you do have a problem with yamltodb
, please open another issue and provide an example.
Sure thing; I was just trying to avoid opening a duplicate :-)