Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t6hZt-00EwVk-7Q for pgsql-hackers@arkaria.postgresql.org; Fri, 01 Nov 2024 02:39:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1t6hZr-008OCx-Dj for pgsql-hackers@arkaria.postgresql.org; Fri, 01 Nov 2024 02:39:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t6hZr-008OCo-2z for pgsql-hackers@lists.postgresql.org; Fri, 01 Nov 2024 02:39:55 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1t6hZl-0046qU-5K for pgsql-hackers@lists.postgresql.org; Fri, 01 Nov 2024 02:39:54 +0000 Received: by mail-ej1-x632.google.com with SMTP id a640c23a62f3a-a9a3dc089d8so235316966b.3 for ; Thu, 31 Oct 2024 19:39:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730428787; x=1731033587; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=iJsYI0RqUJIquPvJ/dX+os13M4HQRI1a5PTbqrgySgA=; b=gtfHDppyh3dA6bG+VlHStrdKOMUuNg+Dz7o+SNghgQMvezTUvrLeGlPqtnjYxyeyvy g8Uo3tw9ddNsjs1dALmksujOW1ISy9tRjyX0D1W6Xzi9ysbAziIaZhMhZdMWFNnp5uEP MvIdCOcjcyZcvS6eUl/u+8qdWBcyM3mXO9dNhPdxk1/e/i1h63/1GaIzKz3yAbmt7OVL EifAbBAxPPs+wskqY1zhEVN2C7Nh9JQeJVomNC/Xfheb/uOUKbzV/6B4fv0r+a9JPkGt E7U2v7ewQfFmcdkKWyUN1qiojG/rQrnJ4Fsfdvp6/vR5scH47OQcQbfir86I5QY1VUaZ JhqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730428787; x=1731033587; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=iJsYI0RqUJIquPvJ/dX+os13M4HQRI1a5PTbqrgySgA=; b=tHrqOuLkqr8Ze8q2L+rNoSSXRlUkW+caXgQN6W+cscnhELuMM82m1p5kc6SVMkO80h /Qq9nnTEhOpC4p5smbGVKl4oUjRA1OFuPMO6o6UM1rT2sfmBW4Jj4MAIVhetvsqtq8mv sjqPlJXpiC/KG4dMe+xZlVfip8zbOukP2RfS10dvtGQgJ5W6T8BKHr5gErDR0GLyt2iM wXaC1e91oWLl1VSr6gdRhJaSIL82taE9XCCKJWd3At6W8RHQbtRPPtowSf/5/7jceefM xzQjEuRjsKctOC3g/xui6yyaX6DRqZPBXrA+5cckdIh/RBDVApxXBDUH8qdCfkGVqIGz xZ6w== X-Forwarded-Encrypted: i=1; AJvYcCU4hSwIdDdSfj4eOSM7IzLbk9w4Dgnf7vf6gJ/o1AZe4YC+XinyEAUbez6zgjHZxXxBF2EpLChHkZMm+Cp0@lists.postgresql.org X-Gm-Message-State: AOJu0Yzu9R1hpd0eP5P9lB44O2pZogK7Wh6HUSPCYthEZKLJ6SYFKBIP AirGsSJh0Q1pTMQiEdxqjRTI13iNHw4329xQzJlLrXg+RPXiqaFkECrS8D72Ba8PJJX/+ATssqV UaAsynZ8fWoPNpNfA7dRzuNxHMy4= X-Google-Smtp-Source: AGHT+IEPlQfFCy09yBSGkzY/I81/XH1KA+e38p8iAcivZurrSY9j97Ju8sqy7ELbho+x4LeLNKiKc2eQpbG30n2OPJc= X-Received: by 2002:a17:907:2d9e:b0:a99:7455:25f2 with SMTP id a640c23a62f3a-a9e6550d89cmr171897566b.39.1730428787234; Thu, 31 Oct 2024 19:39:47 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Tender Wang Date: Fri, 1 Nov 2024 10:39:35 +0800 Message-ID: Subject: Re: Wrong result when enable_partitionwise_join is on if collation of PartitionKey and Column is different. To: Amit Langote Cc: Junwang Zhao , jian he , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="0000000000002c217b0625d0d995" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000002c217b0625d0d995 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Amit Langote =E4=BA=8E2024=E5=B9=B410=E6=9C=8831= =E6=97=A5=E5=91=A8=E5=9B=9B 21:09=E5=86=99=E9=81=93=EF=BC=9A > On Wed, Oct 30, 2024 at 9:36=E2=80=AFPM Junwang Zhao = wrote: > > On Wed, Oct 30, 2024 at 11:58=E2=80=AFAM jian he > wrote: > > > > > > I missed a case when column collation and partition key collation are > > > the same and indeterministic. > > > that should be fine for partition-wise join. > > > so v2 attached. > > > > > > have_partkey_equi_join, match_expr_to_partition_keys didn't do any > > > collation related check. > > > propose v2 change disallow partitionwise join for case when > > > column collation is indeterministic *and* is differ from partition > > > key's collation. > > > > > > the attached partition_wise_join_collation.sql is the test script. > > > you may use it to compare with the master behavior. > > > > What if the partkey collation and column collation are both > deterministic, > > but with different sort order? > > > > I'm not familiar with this part of the code base, but it seems to me th= e > > partition wise join should use partkey collation instead of column > collation, > > because it's the partkey collation that decides which partition a row t= o > > be dispatched. > > > > What Jian proposed is also reasonable but seems another aspect of > $subject? > > I think we should insist that the join key collation and the partition > collation are exactly the same and refuse to match them if they are > not. > > + { > + Oid colloid =3D exprCollation((Node *) expr); > + > + if ((partcoll !=3D colloid) && > + OidIsValid(colloid) && > + !get_collation_isdeterministic(colloid)) > + *coll_incompatiable =3D true; > > I am not quite sure what is the point of checking whether or not the > expression collation is deterministic after confirming that it's not > the same as partcoll. > Me, too. > > Attached 0002 is what I came up with. One thing that's different from > what Jian proposed is that match_expr_to_partition_keys() returns -1 > Agree. (expr not matched to any key) when the collation is also not matched > instead of using a separate output parameter for that. > In have_partkey_equi_join() ... if (exprs_known_equal(root, expr1, expr2, btree_opfamily)) { Oid partcoll2 =3D rel1->part_scheme->partcollation[ipk]; .... I think we should use rel2 here, not rel1. --=20 Thanks, Tender Wang --0000000000002c217b0625d0d995 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
Amit Langote <amitlangote09@gmail.com> =E4=BA=8E2024=E5=B9= =B410=E6=9C=8831=E6=97=A5=E5=91=A8=E5=9B=9B 21:09=E5=86=99=E9=81=93=EF=BC= =9A
On Wed, Oct = 30, 2024 at 9:36=E2=80=AFPM Junwang Zhao <zhjwpku@gmail.com> wrote:
> On Wed, Oct 30, 2024 at 11:58=E2=80=AFAM jian he <jian.universality@gmail.com= > wrote:
> >
> > I missed a case when column collation and partition key collation= are
> > the same and indeterministic.
> > that should be fine for partition-wise join.
> > so v2 attached.
> >
> > have_partkey_equi_join, match_expr_to_partition_keys didn't d= o any
> > collation related check.
> > propose v2 change disallow partitionwise join for=C2=A0 case when=
> > column collation is indeterministic *and* is differ from partitio= n
> > key's collation.
> >
> > the attached partition_wise_join_collation.sql is the test script= .
> > you may use it to compare with the master behavior.
>
> What if the partkey collation and column collation are both determinis= tic,
> but with different sort order?
>
> I'm not familiar with this part of the code base, but it seems to = me the
> partition wise join should use partkey collation instead of column col= lation,
> because it's the partkey collation that decides which partition a = row to
> be dispatched.
>
> What Jian proposed is also reasonable but seems another aspect of $sub= ject?

I think we should insist that the join key collation and the partition
collation are exactly the same and refuse to match them if they are
not.

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Oid=C2=A0 =C2=A0 = =C2=A0colloid =3D=C2=A0 exprCollation((Node *) expr);
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((partcoll !=3D = colloid) &&
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 OidI= sValid(colloid) &&
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !get= _collation_isdeterministic(colloid))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*coll= _incompatiable =3D true;

I am not quite sure what is the point of checking whether or not the
expression collation is deterministic after confirming that it's not the same as partcoll.

Me, too.=C2=A0

Attached 0002 is what I came up with.=C2=A0 One thing that's different = from
what Jian proposed is that match_expr_to_partition_keys() returns -1

Agree.=C2=A0=C2=A0

(expr not matched to any key) when the collation is also not matched
instead of using a separate output parameter for that.

In have_partkey_equi_join()
...
if (exprs_kn= own_equal(root, expr1, expr2, btree_opfamily))
{
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0Oid partcoll2 =3D rel1->part_scheme->partcol= lation[ipk];
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0....
I think we should use rel2 here, not rel1.

=

--
<= div dir=3D"ltr" class=3D"gmail_signature">
Thanks,
Tender Wang
--0000000000002c217b0625d0d995--