Horton

Re: path toward faster partition pruning

David Rowley <david.rowley@2ndquadrant.com>

View thread View on postgr.es
From: David Rowley <david.rowley@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Beena Emerson <memissemerson@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-11-03T04:32:59Z
Lists: pgsql-hackers 
On 31 October 2017 at 21:43, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attached updated version of the patches addressing some of your comments

I've spent a bit of time looking at these but I'm out of time for now.

So far I have noted down the following;

1. This comment seem wrong.

/*
* Since the clauses in rel->baserestrictinfo should all contain Const
* operands, it should be possible to prune partitions right away.
*/

How about PARTITION BY RANGE (a) and SELECT * FROM parttable WHERE a > b; ?
baserestrictinfo in this case will contain a single RestrictInfo with
an OpExpr containing two Var args and it'll come right through that
function too.

2. This code is way more complex than it needs to be.

if (num_parts > 0)
{
int j;

all_indexes = (int *) palloc(num_parts * sizeof(int));
j = 0;
if (min_part_idx >= 0 && max_part_idx >= 0)
{
for (i = min_part_idx; i <= max_part_idx; i++)
all_indexes[j++] = i;
}
if (!bms_is_empty(other_parts))
while ((i = bms_first_member(other_parts)) >= 0)
all_indexes[j++] = i;
if (j > 1)
qsort((void *) all_indexes, j, sizeof(int), intcmp);
}

It looks like the min/max partition stuff is just complicating things
here. If you need to build this array of all_indexes[] anyway, I don't
quite understand the point of the min/max. It seems like min/max would
probably work a bit nicer if you didn't need the other_parts
BitmapSet, so I recommend just getting rid of min/max completely and
just have a BitmapSet with bit set for each partition's index you
need, you'd not need to go to the trouble of performing a qsort on an
array and you could get rid of quite a chunk of code too.

The entire function would then not be much more complex than:

partindexes = get_partitions_from_clauses(parent, partclauses);

while ((i = bms_first_member(partindexes)) >= 0)
{
    AppendRelInfo *appinfo = rel->part_appinfos[i];
    result = lappend(result, appinfo);
}

Then you can also get rid of your intcmp() function too.

3. Following code has the wrong size calculation:

memset(keynullness, -1, PARTITION_MAX_KEYS * sizeof(NullTestType *));

should be PARTITION_MAX_KEYS * sizeof(NullTestType). It might have
worked on your machine if you're compiling as 32 bit.

I'll continue on with the review in the next few days.


-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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.