Horton

Re: [HACKERS] path toward faster partition pruning

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

View thread View on postgr.es
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: jesper.pedersen@redhat.com, David Rowley <david.rowley@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, Amit Langote <amitlangote09@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Beena Emerson <memissemerson@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-04-06T02:03:42Z
Lists: pgsql-hackers

Attachments

On 2018/04/06 8:33, Alvaro Herrera wrote:
>> @@ -1717,8 +1691,8 @@ expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
>>  	 * parentrte already has the root partrel's updatedCols translated to match
>>  	 * the attribute ordering of parentrel.
>>  	 */
>> -	if (!*part_cols_updated)
>> -		*part_cols_updated =
>> +	if (!root->partColsUpdated)
>> +		root->partColsUpdated =
>>  			has_partition_attrs(parentrel, parentrte->updatedCols, NULL);
> 
> Hmm, surely this should be |= to avoid resetting a value set in a
> previous call to this function?

It won't be, no?  We set it only if it hasn't been already.  Note that
there is one PlannerInfo per sub-query, so we determine this information
independently for each sub-query.

> In the previous coding it wasn't
> necessary because it was a local variable ...  (though, isn't it a bit
> odd to have this in PlannerInfo?  seems like it should be in
> resultRelInfo, but then you already have it there so I suppose this one
> does *more*)

Hmm, you'd think that we can figure this out in the executor itself, and
hence don't to have this in PlannerInfo or in ModifyTable.  But IIRC,
during the discussion of the update tuple routing patch, it became clear
that it's best do that here, given the way things are now wrt the timing
of partition/inheritance tree expansion.  An update query may modify the
partition key of a table at any arbitrary level and we have to look at all
the tables in the partition tree in this planning phase anyway, so it's
also the best time to see it if the query's modifiedCols overlaps with the
partition key of some table in the tree.  Once we've found that it does
for some table (most likely the root), we're done, that is, we know we got
some "partColsUpdated".


I realized that I had gotten rid of has_default_part from RelOptInfo but
hadn't deleted a comment about it; attached patch to fix that.

Thanks,
Amit

Commits

  1. Fix assorted partition pruning bugs

  2. Make gen_partprune_steps static

  3. Remove useless 'default' clause

  4. Reorganize partitioning code

  5. Use custom hash opclass for hash partition pruning

  6. Blindly attempt to fix sepgsql tests broken due to 9fdb675fc5.

  7. Attempt to fix endianess issues in new hash partition test.

  8. Faster partition pruning

  9. For partitionwise join, match on partcollation, not parttypcoll.

  10. Revise API for partition bound search functions.

  11. Revise API for partition_rbound_cmp/partition_rbound_datum_cmp.

  12. Fix possible crash in partition-wise join.

  13. Refactor code for partition bound searching

  14. New C function: bms_add_range

  15. Add extensive tests for partition pruning.

  16. Add null test to partition constraint for default range partitions.

  17. Remove BufFile's isTemp flag.

  18. Make OWNER TO subcommand mention consistent

  19. Fix index matching for operators with mixed collatable/noncollatable inputs.