search

dbsrgits / sql-translator

SQL::Translator (SQLFairy)
http://sqlfairy.sourceforge.net/
82 stars 91 forks source link

Fix bugs in Parser::DBI::PostgreSQL data_type, size, and default_value #155

Closed nrdvana closed 1 year ago

nrdvana commented 1 year ago

I'm rather surprised to find bugs this large, so please review and see if I have any wrong assumptions here.

I'm running into cases where postgres round-trip from a DBI connection was generating a lot of syntax errors. It seems to have broken handling of column types with sizes, and broken handling of default values for anything other than numeric constants.

The changes are maybe best described by the changes to unit test 66. You can see that the parser used to store char column size in the data_type instead of ->size field, and have an incorrect value in the ->size field, and store literal DDL as plain strings for the default_value. I changed these to behave like other parsers and put the real size in the size column, and use scalar-refs for any default value that is literal DDL.

rabbiveesh commented 1 year ago

let me review this. Though this makes me wonder if really the quoting mechanism is broken. That's certainly echoed in your other issue, #154

rabbiveesh commented 1 year ago

@nrdvana this looks fine; i assume you've been using it happily since you wrote this PR? If so, i'll get it merged post-haste

nrdvana commented 1 year ago

Yes, the patch is part of one of my projects and "works for me". I was just puzzled whether it could have been working for anyone else pre-patch (because it sure looks like it wouldn't). I'm not sure how to locate such users to ask them though. If you roll it out and someone complains, I'm happy to handle the discussion with them and take the blame :-)

rabbiveesh commented 1 year ago

deal :smile:

rabbiveesh commented 1 year ago

I never ran into any of these issues b/c i turn quoting off

nrdvana commented 1 year ago

If by "quoting" you mean column name quoting, that would be related to #154 where quoting the names of the columns in an index prevents extra info from being appended to the column name. This one is about value quoting, where Postgres reading default values of columns get an extra layer of quotes around them. My only guess would be that nobody is both schema-loading and then ->deploying that same loaded schema?