On 17/10/2020 18:54, Alvaro Herrera wrote:
On 2020-Oct-17, Amit Langote wrote:
As I said in my previous email, I don't see how we can make
initializing the map any lazier than it already is. If a partition
has a different tuple descriptor than the root table, then we know for
sure that any tuples that are routed to it will need to be converted
from the root tuple format to its tuple format, so we might as well
build the map when the ResultRelInfo is built. If no tuple lands into
a partition, we would neither build its ResultRelInfo nor the map.
With the current arrangement, if the map field is NULL, it simply
means that the partition has the same tuple format as the root table.
I see -- makes sense.
It's probably true that there's no performance gain from initializing
them more lazily. But the reasoning and logic around the initialization
is complicated. After tracing through various path through the code, I'm
convinced enough that it's correct, or at least these patches didn't
break it, but I still think some sort of lazy initialization on first
use would make it more readable. Or perhaps there's some other
refactoring we could do.
Perhaps we should have a magic TupleConversionMap value to mean "no
conversion required". NULL could then mean "not initialized yet".
On Fri, Oct 16, 2020 at 11:45 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
BTW it is curious that ExecInitRoutingInfo is called both in
ExecInitPartitionInfo() (from ExecFindPartition when the ResultRelInfo
for the partition is not found) *and* from ExecFindPartition again, when
the ResultRelInfo for the partition *is* found. Doesn't this mean that
ri_PartitionInfo is set up twice for the same partition?
No. ExecFindPartition() directly calls ExecInitRoutingInfo() only for
reused update result relations, that too, only the first time a tuple
lands into such a partition. For the subsequent tuples that land into
the same partition, ExecFindPartition() will be able to find that
ResultRelInfo in the proute->partitions[] array. All ResultRelInfos
in that array are assumed to have been processed by
ExecInitRoutingInfo().
Doh, right, sorry, I was misreading the if/else maze there.
I think that demonstrates my point that the logic is hard to follow :-).
- Heikki
