Skip Menu |
 

This queue is for tickets about the SQL-Translator CPAN distribution.

Report information
The Basics
Id: 70923
Status: new
Priority: 0/
Queue: SQL-Translator

People
Owner: Nobody in particular
Requestors: antony.gelberg [...] gmail.com
philip.allison [...] smoothwall.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Outputs invalid SQL (PostgreSQL)
Download (untitled) / with headers
text/plain 972b
I've attached the diff produced by create_ddl_dir when attempting to upgrade my schema. I can't attach the entire schema but here's the relevant table in 0.006: CREATE TABLE "query" ( "id" serial NOT NULL, "user" integer NOT NULL, "query" text NOT NULL, "shared" boolean NOT NULL, "stamp" timestamp NOT NULL, "language_code" varchar(2) NOT NULL, PRIMARY KEY ("id") ); CREATE INDEX "query_idx_user" on "query" ("user"); and in 0.007: CREATE TABLE "query" ( "id" serial NOT NULL, "user" integer, "query" text NOT NULL, "shared" boolean NOT NULL, "stamp" timestamp NOT NULL, "language_code" varchar(2) NOT NULL, PRIMARY KEY ("id") ); CREATE INDEX "query_idx_user" on "query" ("user"); As you see the only difference is that "user" is now allowed to be null. The first line in the attached file barfs - the second is fine (once I put double quotes round "user"). Sorry I don't have the bandwidth to write a test but hope this is useful info.
Subject: Reask-Schema-0.006-0.007-PostgreSQL.sql
-- Convert schema 'sql/Reask-Schema-0.006-PostgreSQL.sql' to 'sql/Reask-Schema-0.007-PostgreSQL.sql':; BEGIN; ALTER TABLE query DROP CONSTRAINT ; ALTER TABLE query ALTER COLUMN user DROP NOT NULL; COMMIT;
Subject: Incorrect quoting behaviour in PostgreSQL producer (and others?)
Date: Tue, 13 Sep 2011 11:05:44 +0100
To: bug-SQL-Translator [...] rt.cpan.org
From: Philip Allison <philip.allison [...] smoothwall.net>
Download (untitled) / with headers
text/plain 3.3k
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.
Download pg-quote.patch
text/x-diff 708b

Message body is not shown because sender requested not to inline it.

Download signature.asc
application/pgp-signature 198b

Message body not shown because it is not plain text.



This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.