Horton

Re: [HACKERS] path toward faster partition pruning

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: David Rowley <david.rowley@2ndquadrant.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, Jesper Pedersen <jesper.pedersen@redhat.com>, 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-02-23T14:46:11Z
Lists: pgsql-hackers 
On Wed, Feb 21, 2018 at 5:44 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Please find attached updated patches.

Committed 0001 and 0002.

I'm having some difficulty wrapping my head around 0003 because it has
minimal comments and no useful commit message.  I think, though, that
it's actually broken.  Pre-patch, the logic in find_partition_scheme
compares partopfamily, partopcintype, and parttypcoll and then asserts
equality for parttyplen and parttypbyval; not coincidentally,
PartitionSchemeData describes the latter two fields only as "cached
data", so that the segregation of fields in PartitionSchemeData into
two groups exactly matches what find_partition_scheme is actually
doing.  However, with the patch, it turns into a sort of hodgepodge.
parttypid is added into the "cached" section of PartitionSchemeData
and partcollation to the primary section, but both values are
compared, not asserted; parttypcoll moves from the "compared" section
to the "asserted" section but the declaration in PartitionSchemeData
stays where it was.

Moreover, there's no explanation of why this is getting changed.
There's an existing comment that explains the motivation for what the
code does today, which the patch does not modify:

  * We store the opclass-declared input data types instead of the partition key
  * datatypes since the former rather than the latter are used to compare
  * partition bounds. Since partition key data types and the opclass declared
  * input data types are expected to be binary compatible (per ResolveOpClass),
  * both of those should have same byval and length properties.

Obviously, this raises the issue of whether changing this is really
the right thing to do in the first place, but at any rate it's
certainly necessary for the comments to match what the code actually
does.

Also, I find this not very helpful:

+ * The collation of the partition key can differ from the collation of the
+ * underlying column, so we must store this separately.

If the comments about parttypcol and partcollation were clear enough
(and I think they could use some work to distinguish them better),
then this would be pretty much unnecessary -- clearly the only reason
to store two things is if they might be different from each other.

It might be more useful to somehow explain how parttypid and
partsupfunc are intended to be work/be used, but actually I don't
think any satisfactory explanation is possible.  Either we have one
partition scheme per partopcintype -- in which case parttypid is
ill-defined because it could vary among relations with the same
PartitionScheme -- or we have on per parttypid -- in which case,
without some other change, partition-wise join will stop working
between relations with different parttypids but the same
partopcintype.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.