Hello all,
I've discovered some interesting issues in the PostgreSQL producer
module surrounding quoting of table and field names. The "produce"
function calls others in the module with arguments "quote_table_names"
and "quote_field names" set to either the double quote character or
empty, depending on whether or not the options with the same name are
set in the Translator instance itself (lines 206 & 207 of
lib/SQL/Translator/Producer/PostgreSQL.pm). This works perfectly well
when calling produce, but occasionally other parts of the framework -
such as SQL::Translator::Diff - call *other* methods in the producer
modules directly.
When, for example, produce_diff_sql() in Diff calls
alter_drop_constraint() in Producer::PostgreSQL directly, it does *not*
set "quote_table_names" or "quote_field_names" in the arguments, so the
method does *not* encase table or field names in quotes, regardless of
whether or not those options are set on the Translator itself. If one
of the table names happens to be a reserved word, such as "group", then
the resulting SQL contains syntax errors.
This can be worked around by setting "quote_table_names" and
"quote_field_names" directly in producer_args, as follows:
sql_translator_args => {
add_drop_table => 0,
quote_table_names => 1,
quote_field_names => 1,
producer_args => {
quote_table_names => '"',
quote_field_names => '"',
},
},
However, the documentation for the PostgreSQL producer does not mention
these options, and I do not think I should have to set these explicitly
to prevent generation of invalid SQL.
I have attached a documentation patch which adds mention of these
arguments to the PostgreSQL producer, but I am concerned that there may
be wider issues with quoting behaviour, across this and potentially
other Producer implementations. Firstly, table or field names
corresponding to reserved words should always be quoted, otherwise the
resulting SQL will not work - IMHO this is not something that should be
optional*. Secondly, I am confused by the way in which the top-level
Translator instance takes arguments which affect producer behaviour
outside of "producer_args" (the first two occurrences of
"quote_table_names" and "quote_field_names" above), yet the Translator
instance is only passed as an argument to the "produce" method, not all
the other Producer functions. Surely all the arguments which affect
producer behaviour should be in producer_args, or the Translator
instance should be passed into all Producer functions (not just
produce), because it strikes me that the current inconsistency creates
fertile ground for bugs such as this by defining two conflicting ways of
interfacing to producers.
* The module does define a set of reserved words in %reserved, but does
not appear to use it anywhere in the code.
Regards
--
Philip Allison
Senior Developer
philip.allison@smoothwall.net
Smoothwall Ltd
1 John Charles Way, Leeds, LS12 6QA United Kingdom
Telephone: USA: 1 800 959 3760 Europe: +44 (0) 8701 999500
http://www.smoothwall.net
Smoothwall Limited is registered in England, Company Number: 4298247.
Any opinions stated in this message are solely those of the author.
