On Tue, Feb 27, 2018 at 3:03 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attached an updated version in which I incorporated some of the revisions
> that David Rowley suggested to OR clauses handling (in partprune.c) that
> he posted as a separate patch on the run-time pruning thread [1].
Some comments on 0001.
partnatts != part_scheme->partnatts)
continue;
- /* Match the partition key types. */
+ /*
+ * Match the partition key types and partitioning-specific collations.
+ */
We are comparing opfamily and opclass input type as well, but this comment
doesn't explicitly mention those like it mentions collation. Instead, I think
we should just say, "Match partition key type properties"
You had commented on "advanced partition matching code" about asserting that
parsupfuncs also match. Robert too has expressed similar opinion upthread. May
be we should at least try to assert that the function OIDs match.
- Oid *parttypcoll; /* OIDs of collations of partition keys. */
+
+ /*
+ * We store both the collation implied by the partition key's type and the
+ * one specified for partitioning. Values in the former are used as
+ * varcollid in the Vars corresponding to simple column partition keys so
+ * as to make them match corresponding Vars appearing elsewhere in the
+ * query tree. Whereas, the latter is used when actually comparing values
+ * against partition bounds datums, such as, when doing partition pruning.
+ */
+ Oid *parttypcoll;
+ Oid *partcollation;
As you have already mentioned upthread only partcollation is needed, not
parttypcoll.
/* Cached information about partition key data types. */
int16 *parttyplen;
bool *parttypbyval;
+
+ /*
+ * Cached array of partitioning comparison functions' fmgr structs. We
+ * don't compare these when trying to match two partition schemes.
+ */
I think this comment should go away. The second sentence doesn't explain why
and if it did so it should do that in find_partition_scheme() not here.
partsupfunc is another property of partition keys that is cached like
parttyplen, parttypbyval. Why does it deserve a separate comment when others
don't?
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
