remove useless quote_ident

[ADTC]

Why do you consider quote_ident useless? According to Postgres Docs, the use of quote_ident and quote_literal increases the statement safety (for example, if the table name contains spaces) and ensures the dynamically generated statement does not fail from a syntax error.

[ringerc]

I finally took a few minutes to look at this, and it's actually correct. I was not aware that text || regclass applied quoting where required.

create table "I will
hack your');DROP TABLE student;--" (
haha integer
);

SELECT '"I will
hack your'');DROP TABLE student;--"'::regclass::oid;

-- Produces oid 53060 here

regress=> SELECT 'DROP TABLE ' || 53060::oid::regclass;
              ?column?              
------------------------------------
 DROP TABLE "I will                +
 hack your');DROP TABLE student;--"
(1 row)

regress=> SELECT 'DROP TABLE ' || quote_ident(53060::oid::regclass::text);
               ?column?               
--------------------------------------
 DROP TABLE """I will                +
 hack your');DROP TABLE student;--"""
(1 row)

So. Merging.

This is why we include detailed comments on pulls...

[3nids]

the problem is that you can't use this for another schema than public. my pull request solved this, maybe in the wrong direction.

I have no better proposition for this...do you have an idea?

[ADTC]

I agree with the fact that this doesn't work for any schema except public. Indeed, when I attempted to use it for a different schema, it didn't work at first (many statements in the script failed). But I resolved the problem by finding and replacing all instances of public in the script with the correct schema name.

Of course, it was a quick-fix and would obviously not work dynamically.

(Note: This maybe a different issue.)

[3nids]

@ringerc thanks! Sorry for the missing details, there was the issue open #1 , but I forgot to reference properly the pull in it.

Cheers, Denis

[ringerc]

On 01/28/2014 04:02 PM, ADTC wrote:

I agree with the fact that this doesn't work for any schema except |public|. Indeed, when I attempted to use it for a different schema, it didn't work at first (many SQL statements in the script failed). But I resolved the problem by finding and replacing all instances of |public| in the script with the correct schema name.

Of course, it was a quick-fix and would obviously not work dynamically.

Yep. Well, fixed now.

The script should really be an extension, with a control file and --unpackaged upgrade file. That way it can be schema-relocatable, and doesn't have to appear in user dumps.

That's a "later".

Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services