Re: problem with volatile functions in subselects ?

Hello Hackers,

I see the very strange behaviour with the following set of queries:

wsdb=# select na,nb, na::double precision as da, nb::double precision as db from ( select random()::numeric as na,random()::numeric as nb from generate_series(1,2)) as xx;
na | nb | da | db
-------------------+-------------------+-------------------+-------------------
0.756045001445359 | 0.505602368389071 | 0.283893094995941 | 0.160685719065687
0.792114335015469 | 0.416411793053342 | 0.342387438445532 | 0.531201674850286
(2 rows)

On my understanding that should produce the "na" column equal to "da" ?

When I do the same with the select from the table the result is similar:

wsdb=# select na,nb, na::double precision as da, nb::double precision as db from ( select random()::numeric as na,random()::numeric as nb from pg_proc) as xx;
na | nb | da | db
-----------------------+-----------------------+----------------------+----------------------
0.125243402610181 | 0.620239329347498 | 0.64666960465101 | 0.257827353318141
0.934299875951512 | 0.0322264223509591 | 0.96565025298188 | 0.0439542480949099
........

But when I limit the select, I get the expected result.

wsdb=# select na,nb, na::double precision as da, nb::double precision as db from ( select random()::numeric as na,random()::numeric as nb from pg_proc limit 2) as xx;
na | nb | da | db
-------------------+-------------------+-------------------+-------------------
0.543030349324937 | 0.925069289712733 | 0.543030349324937 | 0.925069289712733
0.934251406665077 | 0.292522935332974 | 0.934251406665077 | 0.292522935332974
(2 rows)

Is that a bug, or I'm missing something ?

PG version is 8.1.4 or 8.2dev.

Regards,
Sergey

*******************************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy/Sternberg Astronomical Institute
Tel: +49-6221-528-349
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math(at)sai(dot)msu(dot)ru

"Sergey E. Koposov" <math(at)sai(dot)msu(dot)ru> writes:
> I see the very strange behaviour with the following set of queries:

> wsdb=# select na,nb, na::double precision as da, nb::double precision as db from ( select random()::numeric as na,random()::numeric as nb from generate_series(1,2)) as xx;

The planner "flattens" this query to a single level of SELECT, so what
you effectively have is

select random()::numeric as na,
random()::numeric as nb,
random()::numeric::double precision as da,
random()::numeric::double precision as db
from generate_series(1,2) as xx;

There's been some talk about prohibiting flattening if there are any
volatile functions in the subselect's targetlist, but nothing's been
done about that. Disabling flattening is a sufficiently big hit to
the planner's optimization ability that it shouldn't be done lightly.

regards, tom lane

On 7/30/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Sergey E. Koposov" <math(at)sai(dot)msu(dot)ru> writes:
> > I see the very strange behaviour with the following set of queries:
>
> > wsdb=# select na,nb, na::double precision as da, nb::double precision as db from ( select random()::numeric as na,random()::numeric as nb from generate_series(1,2)) as xx;
>
> The planner "flattens" this query to a single level of SELECT, so what
> you effectively have is
>
> select random()::numeric as na,
> random()::numeric as nb,
> random()::numeric::double precision as da,
> random()::numeric::double precision as db
> from generate_series(1,2) as xx;
>
> There's been some talk about prohibiting flattening if there are any
> volatile functions in the subselect's targetlist, but nothing's been
> done about that. Disabling flattening is a sufficiently big hit to
> the planner's optimization ability that it shouldn't be done lightly.
>
> regards, tom lane
>

http://archives.postgresql.org/pgsql-hackers/2005-10/msg00396.php

maybe a guc?

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

> > There's been some talk about prohibiting flattening if there are any
> > volatile functions in the subselect's targetlist, but nothing's been
> > done about that. Disabling flattening is a sufficiently big hit to
> > the planner's optimization ability that it shouldn't be done lightly.
> >
> > regards, tom lane
> >
>
> http://archives.postgresql.org/pgsql-hackers/2005-10/msg00396.php
>
> maybe a guc?
>

is this worth trying? maybe it's fair to say is to late for 8.2 but
maybe it can be qeued for 8.3...

i think i can do this in 1 or 2 days...

BTW, can you think in a good name for a GUC for this?

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

"Jaime Casanova" <systemguards(at)gmail(dot)com> writes:
>>> There's been some talk about prohibiting flattening if there are any
>>> volatile functions in the subselect's targetlist, but nothing's been
>>> done about that.

> BTW, can you think in a good name for a GUC for this?

I'm not in favor of a GUC for this; we should either do it or not.

If we do it, basically the response to anyone who complains about loss
of performance should be "fix your function to be marked stable or
immutable, as appropriate".

regards, tom lane

On 8/13/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Jaime Casanova" <systemguards(at)gmail(dot)com> writes:
> >>> There's been some talk about prohibiting flattening if there are any
> >>> volatile functions in the subselect's targetlist, but nothing's been
> >>> done about that.
>
> > BTW, can you think in a good name for a GUC for this?
>
> I'm not in favor of a GUC for this; we should either do it or not.
>

me neither, the idea came because seems there wasn't enough
consensus... my opinion always was we have to return right results and
then think on performance...

if someone cares, this is the patch i use for avoiding pulling up of
subqueries containing volatile functions (at least it has worked for
me :)...

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

Based on how small this patch is and the demonstrated desire for this
behavior, can we consider putting this in 8.2, even though we're past
the deadline?

On Sun, Aug 13, 2006 at 10:03:30PM -0500, Jaime Casanova wrote:
> On 8/13/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >"Jaime Casanova" <systemguards(at)gmail(dot)com> writes:
> >>>> There's been some talk about prohibiting flattening if there are any
> >>>> volatile functions in the subselect's targetlist, but nothing's been
> >>>> done about that.
> >
> >> BTW, can you think in a good name for a GUC for this?
> >
> >I'm not in favor of a GUC for this; we should either do it or not.
> >
>
> me neither, the idea came because seems there wasn't enough
> consensus... my opinion always was we have to return right results and
> then think on performance...
>
> if someone cares, this is the patch i use for avoiding pulling up of
> subqueries containing volatile functions (at least it has worked for
> me :)...
>
> --
> regards,
> Jaime Casanova
>
> "Programming today is a race between software engineers striving to
> build bigger and better idiot-proof programs and the universe trying
> to produce bigger and better idiots.
> So far, the universe is winning."
> Richard Cook

> Index: src/backend/optimizer/path/allpaths.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v
> retrieving revision 1.151
> diff -c -p -r1.151 allpaths.c
> *** src/backend/optimizer/path/allpaths.c 10 Aug 2006 02:36:28 -0000 1.151
> --- src/backend/optimizer/path/allpaths.c 14 Aug 2006 02:19:08 -0000
> *************** make_one_rel_by_joins(PlannerInfo *root,
> *** 737,742 ****
> --- 737,745 ----
> * component queries to see if any of them have different output types;
> * differentTypes[k] is set true if column k has different type in any
> * component.
> + *
> + * 4. If the subquery has any volatile functions may not be safe to push
> + * down any quals.
> */
> static bool
> subquery_is_pushdown_safe(Query *subquery, Query *topquery,
> *************** subquery_is_pushdown_safe(Query *subquer
> *** 769,774 ****
> --- 772,782 ----
> topop->colTypes,
> differentTypes);
> }
> +
> + /* Check point 4 */
> + if (contain_volatile_functions(subquery->targetList))
> + return false;
> +
> return true;
> }
>
> Index: src/backend/optimizer/prep/prepjointree.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/optimizer/prep/prepjointree.c,v
> retrieving revision 1.42
> diff -c -p -r1.42 prepjointree.c
> *** src/backend/optimizer/prep/prepjointree.c 12 Aug 2006 20:05:55 -0000 1.42
> --- src/backend/optimizer/prep/prepjointree.c 14 Aug 2006 02:19:11 -0000
> *************** is_simple_subquery(Query *subquery)
> *** 657,662 ****
> --- 657,669 ----
> return false;
>
> /*
> + * Don't pull up a subquery that has any volatile functions in its
> + * targetlist.
> + */
> + if (contain_volatile_functions((Node *) subquery->targetList))
> + return false;
> +
> + /*
> * Hack: don't try to pull up a subquery with an empty jointree.
> * query_planner() will correctly generate a Result plan for a jointree
> * that's totally empty, but I don't think the right things happen if an
> Index: src/include/optimizer/prep.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/optimizer/prep.h,v
> retrieving revision 1.56
> diff -c -p -r1.56 prep.h
> *** src/include/optimizer/prep.h 5 Mar 2006 15:58:57 -0000 1.56
> --- src/include/optimizer/prep.h 14 Aug 2006 02:19:17 -0000
> ***************
> *** 17,23 ****
> #include "nodes/plannodes.h"
> #include "nodes/relation.h"
>
> -
> /*
> * prototypes for prepjointree.c
> */
> --- 17,22 ----

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

--
Jim C. Nasby, Sr. Engineering Consultant jnasby(at)pervasive(dot)com
Pervasive Software http://pervasive.com work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461

Tom Lane wrote:
> "Jaime Casanova" <systemguards(at)gmail(dot)com> writes:
> >>> There's been some talk about prohibiting flattening if there are any
> >>> volatile functions in the subselect's targetlist, but nothing's been
> >>> done about that.
>
> > BTW, can you think in a good name for a GUC for this?
>
> I'm not in favor of a GUC for this; we should either do it or not.
>
> If we do it, basically the response to anyone who complains about loss
> of performance should be "fix your function to be marked stable or
> immutable, as appropriate".

Agreed. Are we doing this, or is it a TODO?

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> If we do it, basically the response to anyone who complains about loss
>> of performance should be "fix your function to be marked stable or
>> immutable, as appropriate".

> Agreed. Are we doing this, or is it a TODO?

It's done:
http://archives.postgresql.org/pgsql-committers/2006-08/msg00358.php

regards, tom lane