Re: should check collations when creating partitioned index

[Peter Eisentraut]

On 20.11.23 17:25, Tom Lane wrote:
Peter Eisentraut <pe...@eisentraut.org> writes:
On 14.11.23 17:15, Tom Lane wrote:
I don't love the patch details though.  It seems entirely wrong to check
this before we check the opclass match.

Not sure why?  The order doesn't seem to matter?

The case that was bothering me was if we had a non-collated type
versus a collated type.  That would result in throwing an error
about collation mismatch, when complaining about the opclass seems
more apropos.  However, if we do this:

I see.  That means we shouldn't raise an error on a mismatch but just do
      if (key->partcollation[i] != collationIds[j])
          continue;

it might not matter much.

Here is an updated patch that works as indicated above.

The behavior if you try to create an index with mismatching collations 
now is that it will skip over the column and complain at the end with 
something like

ERROR:  0A000: unique constraint on partitioned table must include all 
partitioning columns
DETAIL:  UNIQUE constraint on table "t1" lacks column "b" which is part 
of the partition key.

which perhaps isn't intuitive, but I think it would be the same if you 
somehow tried to build an index with different operator classes than the 
partitioning.  I think these less-specific error messages are ok in such 
edge cases.From 869a66c429eb188ceafcbd972b6e46b63fce88f3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 23 Nov 2023 10:37:21 +0100
Subject: [PATCH v2] Check collation when creating partitioned index

When creating a partitioned index, the partition key must be a subset
of the index's columns.  But this currently doesn't check that the
collations between the partition key and the index definition match.
So you can construct a unique index that fails to enforce uniqueness.
(This would most likely involve a nondeterministic collation, so it
would have to be crafted explicitly and is not something that would
just happen by accident.)

This patch adds the required collation check.  As a result, any
previously allowed unique index that has a collation mismatch would no
longer be allowed to be created.

Discussion: 
https://www.postgresql.org/message-id/flat/3327cb54-f7f1-413b-8fdb-7a9dceebb938%40eisentraut.org
---
 src/backend/commands/indexcmds.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0b3b8e98b8..c7ecedbe3b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1011,10 +1011,13 @@ DefineIndex(Oid tableId,
                        {
                                if (key->partattrs[i] == 
indexInfo->ii_IndexAttrNumbers[j])
                                {
-                                       /* Matched the column, now what about 
the equality op? */
+                                       /* Matched the column, now what about 
the collation and equality op? */
                                        Oid                     idx_opfamily;
                                        Oid                     idx_opcintype;
 
+                                       if (key->partcollation[i] != 
collationIds[j])
+                                               continue;
+
                                        if 
(get_opclass_opfamily_and_input_type(opclassIds[j],
                                                                                
                                        &idx_opfamily,
                                                                                
                                        &idx_opcintype))
-- 
2.42.1