Thread: Pseudoconstant quals versus the join removal patch

Pseudoconstant quals versus the join removal patch

From
Tom Lane
Date:
I dug into the 9.0 bug reported here:
http://archives.postgresql.org/pgsql-sql/2010-09/msg00035.php

What is happening is that the planner recognizes that the query is
unsatisfiable, because m.ttype is equated to two different constant
values.  This results in generating a constant-false RestrictInfo,
which ordinarily would result in a dummy plan like this:
               QUERY PLAN                
------------------------------------------Result  (cost=0.00..0.01 rows=1 width=0)  One-Time Filter: false
(2 rows)

Since we want this to be the whole plan, the constant-false RestrictInfo
is forced up to the top of the tree, as per this bit in initsplan.c:
               /* if not below outer join, push it to top of tree */               if (!below_outer_join)
{                  relids =                       get_relids_in_jointree((Node *) root->parse->jointree,
                             false);                   qualscope = bms_copy(relids);               }
 

This results in the constant-false qual being attached to the join plan
level representing the whole query.  There's just one small problem: the
join removal code runs after this point.  Which means we'll never
actually form the joinrel corresponding to the level the qual's been
assigned to, and so we never get around to applying the constant-false
filter.

The comment for remove_rel_from_query says
* We are not terribly thorough here.  We must make sure that the rel is* no longer treated as a baserel, and that
attributesof other baserels* are no longer marked as being needed at joins involving this rel.* In particular, we don't
botherremoving join quals involving the rel from* the joininfo lists; they'll just get ignored, since we will never
forma* join relation at which they could be evaluated.
 

and it's that last bit that is biting us.

I think that it's probably sufficient to make remove_rel_from_query run
through the rel's joininfo list looking for pseudoconstant quals, and
push those back into the joininfo lists with a reduced join list.  I
wonder though if there's a better way, or if there are related bugs
this fix won't cover.  Any thoughts?
        regards, tom lane


Re: Pseudoconstant quals versus the join removal patch

From
Tom Lane
Date:
I wrote:
> I think that it's probably sufficient to make remove_rel_from_query run
> through the rel's joininfo list looking for pseudoconstant quals, and
> push those back into the joininfo lists with a reduced join list.  I
> wonder though if there's a better way, or if there are related bugs
> this fix won't cover.  Any thoughts?

On reflection I decided that outerjoin-delayed quals could probably have
the same problem.  I've changed the code so that all quals not clearly
attached to the specific outer join we're removing will be modified to
ensure they're still evaluated at the right time.
        regards, tom lane


Re: Pseudoconstant quals versus the join removal patch

From
Robert Haas
Date:
On Tue, Sep 14, 2010 at 7:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> I think that it's probably sufficient to make remove_rel_from_query run
>> through the rel's joininfo list looking for pseudoconstant quals, and
>> push those back into the joininfo lists with a reduced join list.  I
>> wonder though if there's a better way, or if there are related bugs
>> this fix won't cover.  Any thoughts?
>
> On reflection I decided that outerjoin-delayed quals could probably have
> the same problem.  I've changed the code so that all quals not clearly
> attached to the specific outer join we're removing will be modified to
> ensure they're still evaluated at the right time.

Thanks for jumping on this.  FTR, I don't read pgsql-sql.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


Re: Pseudoconstant quals versus the join removal patch

From
Robert Haas
Date:
On Tue, Sep 14, 2010 at 8:44 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Sep 14, 2010 at 7:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I wrote:
>>> I think that it's probably sufficient to make remove_rel_from_query run
>>> through the rel's joininfo list looking for pseudoconstant quals, and
>>> push those back into the joininfo lists with a reduced join list.  I
>>> wonder though if there's a better way, or if there are related bugs
>>> this fix won't cover.  Any thoughts?
>>
>> On reflection I decided that outerjoin-delayed quals could probably have
>> the same problem.  I've changed the code so that all quals not clearly
>> attached to the specific outer join we're removing will be modified to
>> ensure they're still evaluated at the right time.
>
> Thanks for jumping on this.  FTR, I don't read pgsql-sql.

One other thought: should we add some of these queries that have
exposed bugs in join removal to the regression tests?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


Re: Pseudoconstant quals versus the join removal patch

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> One other thought: should we add some of these queries that have
> exposed bugs in join removal to the regression tests?

I did.
        regards, tom lane