Re: Evaluate expression at planning time for two more cases - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Evaluate expression at planning time for two more cases
Date
Msg-id 1723765.1599529559@sss.pgh.pa.us
Whole thread Raw
In response to Re: Evaluate expression at planning time for two more cases  (Surafel Temesgen <surafel3000@gmail.com>)
Responses Re: Evaluate expression at planning time for two more cases
Re: Evaluate expression at planning time for two more cases
Re: Evaluate expression at planning time for two more cases
List pgsql-hackers
Surafel Temesgen <surafel3000@gmail.com> writes:
> [ null_check_on_pkey_optimization_v1.patch ]

I took a very brief look at this.

> I don’t add NOT NULL constraint optimization to the patch because cached
> plan is not invalidation in case of a change in NOT NULL constraint

That's actually not a problem, even though some people (including me)
have bandied about such suppositions in the past.  Relying on attnotnull
in the planner is perfectly safe [1].  Plus it'd likely be cheaper as
well as more general than looking up pkey information.  If we did need
to explicitly record the plan's dependency on a constraint, this patch
would be wrong anyhow because it fails to make any such notation about
the pkey constraint it relied on.

The "check_null_side" code you're proposing seems really horrid.
For one thing, it seems quite out of place for eval_const_expressions
to be doing that.  For another, it's wrong in principle because
eval_const_expressions doesn't know which part of the query tree
it's being invoked on, so it cannot know whether outer-join
nullability is an issue.  For another, doing that work over again
from scratch every time we see a potentially optimizable NullTest
looks expensive.  (I wonder whether you have tried to measure the
performance penalty imposed by this patch in cases where it fails
to make any proof.)

I've been doing some handwaving about changing the representation
of Vars, with an eye to making it clear by inspection whether a
given Var is nullable by some lower outer join [2].  If that work
ever comes to fruition then the need for "check_null_side" would
go away.  So maybe we should put this idea on the back burner
until that happens.

I'm not sure what I think about Ashutosh's ideas about doing this
somewhere else than eval_const_expressions.  I do not buy the argument
that it's interesting to do this separately for each child partition.
Child partitions that have attnotnull constraints different from their
parent's are at best a tiny minority use-case, if indeed we allow them
at all (I tend to think we shouldn't).  On the other hand it's possible
that postponing the check would allow bypassing the outer-join problem,
ie if we only do it for quals that have dropped down to the relation
scan level then we don't need to worry about outer join effects.

Another angle here is that eval_const_expressions runs before
reduce_outer_joins, meaning that if it's doing things that depend
on outer-join-ness then it will sometimes fail to optimize cases
that could be optimized.  As a not incidental example, consider

    select ... from t1 left join t2 on (...) where t2.x is not null;

reduce_outer_joins will realize that the left join can be reduced
to a plain join, whereupon (if t2.x is attnotnull) the WHERE clause
really is constant-true --- and this seems like a poster-child case
for it being useful to optimize away the WHERE clause.  But
we won't be able to detect that if we apply the optimization during
eval_const_expressions.  So maybe that's a good reason to do it
somewhere later.

            regards, tom lane

[1] https://www.postgresql.org/message-id/23564.1585885251%40sss.pgh.pa.us
[2] https://www.postgresql.org/message-id/flat/15848.1576515643%40sss.pgh.pa.us



pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: v13: CLUSTER segv with wal_level=minimal and parallel index creation
Next
From: Junfeng Yang
Date:
Subject: 回复: Is it possible to set end-of-data marker for COPY statement.