Pyrseas icon indicating copy to clipboard operation
Pyrseas copied to clipboard

Table named with PG reserved word and constraint causes error in dbtoyaml

Open jmafc opened this issue 7 years ago • 5 comments

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.

jmafc avatar Nov 15 '17 01:11 jmafc

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.

rmg avatar Dec 04 '19 22:12 rmg

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.

jmafc avatar Dec 04 '19 22:12 jmafc

Sorry, I misread the issue title. I am indeed talking about yamltodb.

rmg avatar Dec 04 '19 22:12 rmg

OK, so if you do have a problem with yamltodb, please open another issue and provide an example.

jmafc avatar Dec 04 '19 23:12 jmafc

Sure thing; I was just trying to avoid opening a duplicate :-)

rmg avatar Dec 04 '19 23:12 rmg