Re: inconsistent results querying table partitioned by date - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: inconsistent results querying table partitioned by date |
Date | |
Msg-id | 23372.1557966530@sss.pgh.pa.us Whole thread Raw |
In response to | Re: inconsistent results querying table partitioned by date (David Rowley <david.rowley@2ndquadrant.com>) |
Responses |
Re: inconsistent results querying table partitioned by date
(David Rowley <david.rowley@2ndquadrant.com>)
|
List | pgsql-bugs |
David Rowley <david.rowley@2ndquadrant.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
pgsql-bugs by date: