Thread: BUG #5358: Throwing unexpected ERROR
The following bug has been logged online:
Bug reference: 5358
Logged by: Gurjeet Singh
Email address: singh.gurjeet@gmail.com
PostgreSQL version: 8.4.2
Operating system: Windows Vista 64bit
Description: Throwing unexpected ERROR
Details:
I am using Postgres Plus Standard Server
version: PostgreSQL 8.4.2, compiled by Visual C++ build 1400, 32-bit
create table public.test( a bytea, c text )
select relname, pg_relation_size( 'public."' || relname || '"' )/1024
from (select distinct relname
from (select r.relname, c.attname, c.attnotnull, t.typname
from pg_namespace as s, pg_class as r, pg_attribute as c, pg_type as t
where s.oid = r.relnamespace
and r.oid = c.attrelid
and c.atttypid = t.oid
and s.nspname = 'public'
and t.typname in ('bytea', 'text') ) as s1
) as s2
where pg_relation_size( 'public."' || relname || '"' ) <> 0;
ERROR: relation "public.pg_type" does not exist
********** Error **********
ERROR: relation "public.pg_type" does not exist
SQL state: 42P01
When I comment out the last WHERE clause, the query runs fine.
It seems that the WHERE predicate is being pushed into the subqueries a bit
too soon.
Here's the EXPLAIN:
Subquery Scan s2 (cost=123.50..124.23 rows=21 width=64)
-> HashAggregate (cost=123.50..123.71 rows=21 width=64)
-> Nested Loop (cost=10.93..123.44 rows=21 width=64)
Join Filter: (r.relnamespace = s.oid)
-> Seq Scan on pg_namespace s (cost=0.00..1.08 rows=1
width=4)
Filter: (nspname = 'public'::name)
-> Nested Loop (cost=10.93..121.03 rows=107 width=68)
-> Hash Join (cost=10.93..82.03 rows=107 width=4)
Hash Cond: (c.atttypid = t.oid)
-> Seq Scan on pg_attribute c (cost=0.00..61.57
rows=2257 width=8)
-> Hash (cost=10.90..10.90 rows=2 width=4)
-> Seq Scan on pg_type t (cost=0.00..10.90
rows=2 width=4)
Filter: (typname = ANY
('{bytea,text}'::name[]))
-> Index Scan using pg_class_oid_index on pg_class r
(cost=0.00..0.35 rows=1 width=72)
Index Cond: (r.oid = c.attrelid)
Filter: (pg_relation_size(((('public."'::text ||
(r.relname)::text) || '"'::text))::regclass, 'main'::text) <> 0)
Best regards,
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> select relname, pg_relation_size( 'public."' || relname || '"' )/1024
> from (select distinct relname
> from (select r.relname, c.attname, c.attnotnull, t.typname
> from pg_namespace as s, pg_class as r, pg_attribute as c, pg_type as t
> where s.oid = r.relnamespace
> and r.oid = c.attrelid
> and c.atttypid = t.oid
> and s.nspname = 'public'
> and t.typname in ('bytea', 'text') ) as s1
> ) as s2
> where pg_relation_size( 'public."' || relname || '"' ) <> 0;
> ERROR: relation "public.pg_type" does not exist
That approach to generating a textual name for a relation is really
pretty unworkable: it's on the hairy edge of being vulnerable to
SQL injection attacks, not to mention being inefficient and unwieldy.
Just pass r.oid to pg_relation_size, instead.
regards, tom lane
On Tue, Mar 2, 2010 at 7:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> > select relname, pg_relation_size( 'public."' || relname || '"' )/1024
> > from (select distinct relname
> > from (select r.relname, c.attname, c.attnotnull, t.typname
> > from pg_namespace as s, pg_class as r, pg_attribute as c,
> pg_type as t
> > where s.oid = r.relnamespace
> > and r.oid = c.attrelid
> > and c.atttypid = t.oid
> > and s.nspname = 'public'
> > and t.typname in ('bytea', 'text') ) as s1
> > ) as s2
> > where pg_relation_size( 'public."' || relname || '"' ) <> 0;
>
> > ERROR: relation "public.pg_type" does not exist
>
> That approach to generating a textual name for a relation is really
> pretty unworkable: it's on the hairy edge of being vulnerable to
> SQL injection attacks, not to mention being inefficient and unwieldy.
> Just pass r.oid to pg_relation_size, instead.
>
I have gotten on to that path already, thanks for the advice.
This query will never be used by an application, so no fear of SQL injection
there. I was in the middle of a migration effort when I brewed this query.
The main inner query is what I started with to migrate only specific tables,
and the started slapping on outer queries to monitor the amount of data
already transferred. So I was rather surprised to see this error at a stage
where I did not expect it to fail.
IMHO the outer-most WHERE clause is being pushed through the subqueries when
it should not be. I tried to stop the optimizer from doing that and it seems
putting a LIMIT clause on S1 subquery make Postgres happy.
select relname, pg_relation_size( 'public."' || relname || '"' )/1024
from (select distinct relname
from (select r.relname, c.attname, c.attnotnull, t.typname
from pg_namespace as s, pg_class as r, pg_attribute as c,
pg_type as t
where s.oid = r.relnamespace
and r.oid = c.attrelid
and c.atttypid = t.oid
and s.nspname = 'public'
and t.typname in ('bytea', 'text') ) as s1 limit 1000
) as s2
where pg_relation_size( 'public."' || relname || '"' ) <> 0 ;
From SQL perspective there should be no difference between this query and
the one in the first post since there's only one qualifying record.
Predicate push-down is definitely a good optimization, but it should not
affect the result-set. I have no idea how to tell optimizer to stop such
push-downs.
I am leaning towards marking this as a bug.
Best regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.enterprisedb.com
singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device
On Tue, Mar 2, 2010 at 10:24 PM, Gurjeet Singh <singh.gurjeet@gmail.com>wrote:
> On Tue, Mar 2, 2010 at 7:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> "Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
>> > select relname, pg_relation_size( 'public."' || relname || '"' )/1024
>> > from (select distinct relname
>> > from (select r.relname, c.attname, c.attnotnull, t.typname
>> > from pg_namespace as s, pg_class as r, pg_attribute as c,
>> pg_type as t
>> > where s.oid = r.relnamespace
>> > and r.oid = c.attrelid
>> > and c.atttypid = t.oid
>> > and s.nspname = 'public'
>> > and t.typname in ('bytea', 'text') ) as s1
>> > ) as s2
>> > where pg_relation_size( 'public."' || relname || '"' ) <> 0;
>>
>> > ERROR: relation "public.pg_type" does not exist
>>
>> That approach to generating a textual name for a relation is really
>> pretty unworkable: it's on the hairy edge of being vulnerable to
>> SQL injection attacks, not to mention being inefficient and unwieldy.
>> Just pass r.oid to pg_relation_size, instead.
>>
>
> I have gotten on to that path already, thanks for the advice.
>
> This query will never be used by an application, so no fear of SQL
> injection there. I was in the middle of a migration effort when I brewed
> this query. The main inner query is what I started with to migrate only
> specific tables, and the started slapping on outer queries to monitor the
> amount of data already transferred. So I was rather surprised to see this
> error at a stage where I did not expect it to fail.
>
> IMHO the outer-most WHERE clause is being pushed through the subqueries
> when it should not be. I tried to stop the optimizer from doing that and it
> seems putting a LIMIT clause on S1 subquery make Postgres happy.
>
>
> select relname, pg_relation_size( 'public."' || relname || '"' )/1024
> from (select distinct relname
> from (select r.relname, c.attname, c.attnotnull, t.typname
> from pg_namespace as s, pg_class as r, pg_attribute as c,
> pg_type as t
> where s.oid = r.relnamespace
> and r.oid = c.attrelid
> and c.atttypid = t.oid
> and s.nspname = 'public'
> and t.typname in ('bytea', 'text') ) as s1 limit 1000
>
> ) as s2
> where pg_relation_size( 'public."' || relname || '"' ) <> 0 ;
>
> From SQL perspective there should be no difference between this query and
> the one in the first post since there's only one qualifying record.
> Predicate push-down is definitely a good optimization, but it should not
> affect the result-set. I have no idea how to tell optimizer to stop such
> push-downs.
>
I just realized that it is the subquery pull-up that is leading to this
problem, not predicate push-down. Sleeping over it does really help I guess
:)
So instead of the LIMIT 1000, OFFSET 0 clause is the right choice for
preventing subquery pull-up without affecting the results.
I don't think the optimizer has the push-down capabiity; I may be wrong.
> I am leaning towards marking this as a bug.
>
>
Best regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.enterprisedb.com
singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device
On Wed, Mar 3, 2010 at 7:29 AM, Gurjeet Singh <singh.gurjeet@gmail.com> wrote: > I just realized that it is the subquery pull-up that is leading to this > problem, not predicate push-down. Sleeping over it does really help I guess > :) > > So instead of the LIMIT 1000, OFFSET 0 clause is the right choice for > preventing subquery pull-up without affecting the results. > > I don't think the optimizer has the push-down capabiity; I may be wrong. Maybe I'm just dense, but I don't understand what you're complaining about here. The SELECT DISTINCT already acts as an optimization fence, so why would you need another one? And what problem would you expect it to solve? ...Robert
On Wed, Mar 3, 2010 at 8:37 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Mar 3, 2010 at 7:29 AM, Gurjeet Singh <singh.gurjeet@gmail.com> > wrote: > > I just realized that it is the subquery pull-up that is leading to this > > problem, not predicate push-down. Sleeping over it does really help I > guess > > :) > > > > So instead of the LIMIT 1000, OFFSET 0 clause is the right choice for > > preventing subquery pull-up without affecting the results. > > > > I don't think the optimizer has the push-down capabiity; I may be wrong. > > Maybe I'm just dense, but I don't understand what you're complaining > about here. The SELECT DISTINCT already acts as an optimization > fence, so why would you need another one? And what problem would you > expect it to solve? > > I am complaining about the ERROR when I don't specify OFFSET or LIMIT. The query isn't relevant. It is there just to illustrate the fact that two supposedly equivalent forms of a query are not treated equivalent after all by Postgres. You don't put that OFFSET clause, you get an ERROR. You put in that OFFSET clause and you get proper results. I hope my complain is clearer now. Best regards, -- gurjeet.singh @ EnterpriseDB - The Enterprise Postgres Company http://www.enterprisedb.com singh.gurjeet@{ gmail | yahoo }.com Twitter/Skype: singh_gurjeet Mail sent from my BlackLaptop device
On Wed, Mar 3, 2010 at 8:48 AM, Gurjeet Singh <singh.gurjeet@gmail.com> wro= te: > On Wed, Mar 3, 2010 at 8:37 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> >> On Wed, Mar 3, 2010 at 7:29 AM, Gurjeet Singh <singh.gurjeet@gmail.com> >> wrote: >> > I just realized that it is the subquery pull-up that is leading to this >> > problem, not predicate push-down. Sleeping over it does really help I >> > guess >> > :) >> > >> > So instead of the LIMIT 1000, OFFSET 0 clause is the right choice for >> > preventing subquery pull-up without affecting the results. >> > >> > I don't think the optimizer has the push-down capabiity; I may be wron= g. >> >> Maybe I'm just dense, but I don't understand what you're complaining >> about here. =A0The SELECT DISTINCT already acts as an optimization >> fence, so why would you need another one? =A0And what problem would you >> expect it to solve? >> > > I am complaining about the ERROR when I don't specify OFFSET or LIMIT. > > The query isn't relevant. It is there just to illustrate the fact that two > supposedly equivalent forms of a query are not treated equivalent after a= ll > by Postgres. > > You don't put that OFFSET clause, you get an ERROR. You put in that OFFSET > clause and you get proper results. > > I hope my complain is clearer now. It does seem a little weird, but I don't think we're likely to change the behavior. The optimizer is allowed to reorder quals, and I don't think we want to change that. Consider a very large table which has an index on column b but not on column a, and the following query: SELECT * FROM some_huge_table WHERE a =3D 1 AND b =3D 1 All other things being equal, we'll want to execute this query by doing an index scan for rows with b =3D 1 and then checking whatever comes back to see whether we also have a =3D 1. If we insisted that a =3D 1 had to be evaluated first, we'd have to scan the whole table. Normally this kind of reordering doesn't actually affect the result of the query because normally the quals that are being evaluated don't have any side-effects, but in your query you've chosen something that can throw an exception, so it's user-visible. ...Robert
Robert Haas <robertmhaas@gmail.com> writes: > Normally this kind of reordering doesn't actually affect the result of > the query because normally the quals that are being evaluated don't > have any side-effects, but in your query you've chosen something that > can throw an exception, so it's user-visible. There's a pretty explicit disclaimer here: http://www.postgresql.org/docs/8.4/static/sql-expressions.html#SYNTAX-EXPRESS-EVAL that evaluation order is not guaranteed for WHERE clauses. While we could try to tighten that up, I'll bet quite a lot that the howls of pain from people whose queries stopped getting optimized would drown out anyone who thought it was an improvement. In this particular case I think you could make it better by putting the unsafe function call in a subselect's output list, and placing the restrictive condition in a WHERE clause above the subselect. However, that's only safe because we have restrictions forbidding folding of subselects whose output lists include volatile functions. The problem here is *not* that pg_relation_size is volatile in the normal sense, it's the possibility of throwing error on its input; so the fact that the problem goes away seems a bit accidental. I don't know quite what we might do about that. Marking functions as "could throw error" doesn't seem especially helpful, because the set of functions not so markable is durn close to empty. regards, tom lane