Re: inconsistent results querying table partitioned by date

David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
> Thinking about this more, if we're now making it so a forplanner =
> true set of steps requires all values being compared to the partition
> key to be Const, then anything that's not Const must require run-time
> pruning. That means the:

> else if (func_volatile(fnoid) != PROVOLATILE_IMMUTABLE)

> in analyze_partkey_exprs() can simply become "else".

Um ... what about a merely-stable comparison operator with a Const?

> I think it would be nicer if we made match_clause_to_partition_key()
> do everything that analyze_partkey_exprs() currently does, but if we
> change that "else if" to just "else" then the only advantage is not
> having to make another pass over the pruning steps. We'd have to make
> it so match_clause_to_partition_key() did all the pull_exec_paramids()
> stuff and add some new fields to PartitionPruneStepOp to store that
> info. We'd also need to store hasexecparam in the
> PartitionPruneStepOp and change the signature of
> partkey_datum_from_expr() so we could pass that down as we'd no longer
> have context->exprhasexecparam... Well, we could keep that, but if
> we're trying to not loop over the steps again then we'll need to store
> that value in the step when it's created rather than put it in
> PartitionPruneContext.

The loop over steps, per se, isn't that expensive --- but extra syscache
lookups are. Or at least that's my gut feeling about it. If we just had
match_clause_to_partition_key mark the steps as being plan-time executable
or not, we could avoid the repeat lookup.

>> * Teach match_clause_to_partition_key to not emit plan-time-usable
>> quals at all if it is called with forplanner = false, or

> That's not really possible. Imagine a table partitioned by HASH(a,b)
> and WHERE a = $1 and b = 10; we can't do any pruning during planning
> here, and if we only generate steps containing "a = $1" for run-time,
> then we can't do anything there either.

That's an interesting example, but how does it work now? I did not
observe any code in there that seems to be aware of cross-dependencies
between steps. Presumably somewhere we are combining those hash quals,
so couldn't we mark the combined step properly?

>> Perhaps this implies too much code churn/restructuring to be reasonable
>> to consider right now, but I thought I'd ask. It sure seems like this
>> work is being done in a pretty inefficient and duplicative way.

> I certainly think the above would be cleaner and I don't mind working
> on it, but if it's too much churn for this time of year then I'd
> rather not waste time writing a patch that's going to get rejected.

Yeah, I'm leery of adding much new complexity right now, but maybe
adding a flag field to save a syscache lookup wouldn't be out of line.

regards, tom lane