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.96) (envelope-from ) id 1wD3b8-002YxX-17 for pgsql-hackers@arkaria.postgresql.org; Wed, 15 Apr 2026 17:00:18 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wD3b7-0010nK-1u for pgsql-hackers@arkaria.postgresql.org; Wed, 15 Apr 2026 17:00:17 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wD3b7-0010nC-0K for pgsql-hackers@lists.postgresql.org; Wed, 15 Apr 2026 17:00:17 +0000 Received: from mail-vs1-xe34.google.com ([2607:f8b0:4864:20::e34]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wD3b4-00000001BCL-2gYl for pgsql-hackers@lists.postgresql.org; Wed, 15 Apr 2026 17:00:16 +0000 Received: by mail-vs1-xe34.google.com with SMTP id ada2fe7eead31-6058ac0ebceso4701639137.2 for ; Wed, 15 Apr 2026 10:00:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1776272414; cv=none; d=google.com; s=arc-20240605; b=exZM7FQ0/LVB9qMsspyCT0OIvEosOMlm0gq59Xoyx9qrntKCpI/G0CiMuS0p5hC1Is qI5UU6JhX51Jel18c93c1xVUgzOu7n90CS4XyW2gQkBU148DktndmKKjE+bVhlfO9d8h LPwDGecr73M1GaU2E1j+RSMOCM57eF83DKUG9aa+hg6e1kxGEWxZUACF2SYPMdVm1TbI aXVHoWoS731FlAOkyVkbXwqNBtiq7shsoD0AxfUEqXkJY1UH6Gv2VhBnZUWtx9CgECmW LBSIUZi7dISf5Y5LOQLa21Mi/C6J2c0u91QOjc9s8IlQ1ChvehTfhNWUnt9LRGPpTL53 c2aw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=K1QBvDwyFedp5hLJ+jT8KP03u0DWcbJWPAwBtgq+G1I=; fh=U3Qh24i1saGPbAZlGeoTA7UDYyNs0NIR4u1bfrWLCwc=; b=kKwo16dvMPPzlEBkERe2O+DmmU0v0C0s1lRU/uPlTJX7zH5awbHlHUiS9g+IrX9yF5 0VTI0EX+yjIeAsaYeXGYlUbUCVoJ2lMBQ/1trCViGJjt08KMvORZqwnWdvbUc1RN30xf /auI+Dgpr8CVLWbo+YrR7fH10TOgkNhDjbyPEICEj5uyWeUs2iAcUdlXL5RCSTaal/7f gGLkzS308lxvSI/oRzLADh8tS9qYqypIBEtvD6ls33eXin8iUZuN9V89zYf/ZrtLR7pE cF0mS0cjrtYpl+mpNyj9303dLEXjQJOXli3roBDczVT4VtGDBX2MCE8ikQvpOc4+HdzU OCFQ==; darn=lists.postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1776272414; x=1776877214; 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=K1QBvDwyFedp5hLJ+jT8KP03u0DWcbJWPAwBtgq+G1I=; b=jQEqCL3RZnTeLUrJw3s9aRCXoKCTeVIeCkzCCaMgHZeH9Ru1JyBnUcjIvVbRDDG9/W 4t3RVUcEyobQjtRm4Q4yR91rls9v2241NDhXlO4UF7b1ttceZRITyMIC+tA44IC2tHne 4COWw6C4oheylzwB9GdJ0NQzDum9vyC/xbjxtK/Xvo20WUQaUCXtFEXtbTCWBsXA9M7K ojgD88pxJXv0H3aoFIMIpWrpEHKYr83QA0T+benQ0NEHo4WouFeLIrBF+6+GvX+AkeSa J4kixxZi7szGKZI5ph/YhKDYIf79ss3/JRQgTSJSmFAGDMDL5c7rJ11qhEvx+bJV14OG Pr8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776272414; x=1776877214; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=K1QBvDwyFedp5hLJ+jT8KP03u0DWcbJWPAwBtgq+G1I=; b=AYGWhNHDkh7CSKBWYeOU7IqUivTlm7jPqYgtITbjziOoUt3zaLn2W09VPfclKVVDsT mHTDtQe3d62MsLAg7HJDNtNLebrmikBHW0O2iI585K//7oDZ/FgC3O9XCRDlcWV3LFrS HtXqnEt+OFwl7QzlVQK1/zEdIOn3893pbuzH+bCM4zLdFIA+28vCvIzrfCXxr9gzTAim xPPI8yG66QpRrHyRN6ntgQteJ2lMmyJSl/1CI35s5r4vFspFE0sa/xW8LJB324R6PIZJ bS+K3TwmcodMWXCiOHo7r3lS4w7BFKLfSJDtAE3W1EEYlGKgw43LTpK4/rPUatX8O5M7 E6sw== X-Gm-Message-State: AOJu0Ywi5JUGESrlO+fmPKNv4lgz5ukyg78JipFyWVBiaZpO0+VrLEfD IWO+2waBDCj8a8gl4bZYPNkHgk6sNH87J7dohq88ozmJ37a6+FdP3p0vDjo0/H3uxhWhpT+lpBa Jk79v+dykhYz20YKe6BI294MLAPqejs4= X-Gm-Gg: AeBDieuMn9JgXkWIM4Vnj+KbFilKJQpdPs6d30LiEveC0YTas3pAAvmVGjyMsY7wbdN ZAwu8jlNURns38pUc4QyC0wtQ1MeKqJ7maL5TvEx3OMlzQzg/dLRSXgkg0JWVimwOgjB8X7OxBC Wla2XSqq30d1YT9O8kU6gErt6RuiOOOHQ+TPCWn1kUJpjilLhqLj+wcCD+nEiGhZzoKgn0IDnF+ ZJv9tsv6SPnMxXvRPJlLuaWdTLaTBIVBUwG6NJDPTX5n3LkqtoMS+OdCJDxiZZT56/RPZREWJhX GhPk7nMHVslnZNsG4g== X-Received: by 2002:a05:6102:20c2:b0:60c:bca6:8198 with SMTP id ada2fe7eead31-60cbca68830mr5942579137.10.1776272413995; Wed, 15 Apr 2026 10:00:13 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: SATYANARAYANA NARLAPURAM Date: Wed, 15 Apr 2026 10:00:02 -0700 X-Gm-Features: AQROBzCAh51Y4vyrVTBKAH3Cl3vvL1yaVFUp76WMLxKJcxAtHknvkD_ZRg5tx3U Message-ID: Subject: Re: Bug: Missing collation assignment for GRAPH_TABLE COLUMNS expressions To: Ashutosh Bapat Cc: PostgreSQL Hackers , Peter Eisentraut Content-Type: multipart/alternative; boundary="00000000000042b44d064f82a699" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000042b44d064f82a699 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ashutosh, On Wed, Apr 15, 2026 at 8:07=E2=80=AFAM Ashutosh Bapat wrote: > On Fri, Apr 10, 2026 at 11:36=E2=80=AFPM SATYANARAYANA NARLAPURAM > wrote: > > > > Hi Ashutosh, > > > > On Fri, Apr 10, 2026 at 9:25=E2=80=AFAM Ashutosh Bapat < > ashutosh.bapat.oss@gmail.com> wrote: > >> > >> Hi Satya, > >> Thanks for the report and patch. > >> > >> On Fri, Apr 10, 2026 at 9:12=E2=80=AFPM SATYANARAYANA NARLAPURAM > >> wrote: > >> > > >> > Hi hackers, > >> > > >> > GRAPH_TABLE COLUMNS expressions that involve collation-dependent > functions or operators fail with: > >> > > >> > ERROR: could not determine which collation to use for upper() > function > >> > HINT: Use the COLLATE clause to set the collation explicitly. > >> > > >> > Setup: > >> > > >> > CREATE TABLE vtx (id int PRIMARY KEY, name text); > >> > CREATE TABLE edg (id int PRIMARY KEY, > >> > src int REFERENCES vtx(id), > >> > dst int REFERENCES vtx(id)); > >> > INSERT INTO vtx VALUES (1,'Alice'),(2,'Bob'),(3,'Carol'); > >> > INSERT INTO edg VALUES (1,1,2),(2,2,3); > >> > > >> > CREATE PROPERTY GRAPH g > >> > VERTEX TABLES (vtx KEY (id)) > >> > EDGE TABLES (edg KEY (id) > >> > SOURCE KEY (src) REFERENCES vtx (id) > >> > DESTINATION KEY (dst) REFERENCES vtx (id)); > >> > > >> > postgres=3D# SELECT * FROM GRAPH_TABLE (g > >> > MATCH (a IS vtx)-[e IS edg]->(b IS vtx) COLUMNS (upper(a.name) AS > src_upper)); > >> > ERROR: could not determine which collation to use for upper() > function > >> > HINT: Use the COLLATE clause to set the collation explicitly. > >> > > >> > > >> > In transformRangeGraphTable(), the COLUMNS transformation loop calls > transformExpr() > >> > on each column expression but omits the subsequent > assign_expr_collations() call. Both > >> > WHERE clause transformation sites in parse_graphtable.c correctly > include it. > >> > > >> > Attached a patch to fix this. > >> > >> I think the fix is in the right direction. It's better to call > >> assign_expr_collation only once on all the columns at the end of loop > >> of rgt->columns, just like assign_expr_collation is called on all the > >> conditions in WHERE clause once > > > > > > Addressed this in v2 patch. > > > > If we call assign_expr_collations() on a list, the List expression > also gets a collation, which isn't what we want here. We want to > assign collations to the individual COLUMNs expression independently. > assign_list_collations() is better suited for that. I must say that > your earlier patch had got it right in this regard since it was > calling assign_expr_collations independently on each COLUMNs > expression. However, considering that an all properties reference is > replaced by a list of GraphPropertyRefs in place, I think calling > assign_list_collations() once on all COLUMNs expressions is a > future-proof fix. This is also inline with how collations are assigned > to targetlist expressions in a Query. > Nice catch! > > >> > >> > >> > >> Good to see tests also included in the patch. Do we need all three > >> queries? Also those queries should be placed near the section "-- test > >> collation specified in the expression" and add a query for explicit > >> collation in COLUMNs expression. > > > > > > Removed two tests and moved the test. Explicit collate test already > exists. > > I merged this test into an existing test to avoid adding yet another > query in the file that has many many queries already. Yes, an explicit > collation test is not needed separately. > Thanks! > > While at it I added comments to explain why we aren't performing > en-masse collation assignment on a GraphTable or GraphPathPattern. > > Please let me know what you think of the attached patch. > This LGTM. I reran the tests and all of them are passing. Thanks, Satya --00000000000042b44d064f82a699 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Ashutosh,

On Wed, Apr 15, 202= 6 at 8:07=E2=80=AFAM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
On Fri, Apr 10, 2026 at 11:36=E2= =80=AFPM SATYANARAYANA NARLAPURAM
<satyanar= lapuram@gmail.com> wrote:
>
> Hi Ashutosh,
>
> On Fri, Apr 10, 2026 at 9:25=E2=80=AFAM Ashutosh Bapat <ashutosh.bapat.oss@g= mail.com> wrote:
>>
>> Hi Satya,
>> Thanks for the report and patch.
>>
>> On Fri, Apr 10, 2026 at 9:12=E2=80=AFPM SATYANARAYANA NARLAPURAM >> <satyanarlapuram@gmail.com> wrote:
>> >
>> > Hi hackers,
>> >
>> > GRAPH_TABLE COLUMNS expressions that involve collation-depend= ent functions or operators fail with:
>> >
>> >=C2=A0 =C2=A0ERROR: could not determine which collation to use= for upper() function
>> >=C2=A0 =C2=A0HINT: Use the COLLATE clause to set the collation= explicitly.
>> >
>> > Setup:
>> >
>> >=C2=A0 =C2=A0CREATE TABLE vtx (id int PRIMARY KEY, name text);=
>> >=C2=A0 =C2=A0CREATE TABLE edg (id int PRIMARY KEY,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0src int REFERENCES vtx(id),
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0dst int REFERENCES vtx(id));
>> >=C2=A0 =C2=A0INSERT INTO vtx VALUES (1,'Alice'),(2,= 9;Bob'),(3,'Carol');
>> >=C2=A0 =C2=A0INSERT INTO edg VALUES (1,1,2),(2,2,3);
>> >
>> >=C2=A0 =C2=A0CREATE PROPERTY GRAPH g
>> >=C2=A0 =C2=A0 =C2=A0VERTEX TABLES (vtx KEY (id))
>> >=C2=A0 =C2=A0 =C2=A0EDGE TABLES (edg KEY (id)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0SOURCE KEY (src) REFERENCES vtx (id= )
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0DESTINATION KEY (dst) REFERENCES vt= x (id));
>> >
>> > postgres=3D# SELECT * FROM GRAPH_TABLE (g
>> >=C2=A0 =C2=A0MATCH (a IS vtx)-[e IS edg]->(b IS vtx) COLUMN= S (upper(a.n= ame) AS src_upper));
>> > ERROR:=C2=A0 could not determine which collation to use for u= pper() function
>> > HINT:=C2=A0 Use the COLLATE clause to set the collation expli= citly.
>> >
>> >
>> > In transformRangeGraphTable(), the COLUMNS transformation loo= p calls transformExpr()
>> > on each column expression but omits the subsequent assign_exp= r_collations() call.=C2=A0 Both
>> > WHERE clause transformation sites in parse_graphtable.c corre= ctly include it.
>> >
>> > Attached a patch to fix this.
>>
>> I think the fix is in the right direction. It's better to call=
>> assign_expr_collation only once on all the columns at the end of l= oop
>> of rgt->columns, just like assign_expr_collation is called on a= ll the
>> conditions in WHERE clause once
>
>
> Addressed this in v2 patch.
>

If we call assign_expr_collations() on a list, the List expression
also gets a collation, which isn't what we want here. We want to
assign collations to the individual COLUMNs expression independently.
assign_list_collations() is better suited for that. I must say that
your earlier patch had got it right in this regard since it was
calling assign_expr_collations independently on each COLUMNs
expression. However, considering that an all properties reference is
replaced by a list of GraphPropertyRefs in place, I think calling
assign_list_collations() once on all COLUMNs expressions is a
future-proof fix. This is also inline with how collations are assigned
to targetlist expressions in a Query.

N= ice catch!
=C2=A0

>>
>>
>>
>> Good to see tests also included in the patch. Do we need all three=
>> queries? Also those queries should be placed near the section &quo= t;-- test
>> collation specified in the expression" and add a query for ex= plicit
>> collation in COLUMNs expression.
>
>
> Removed two tests and moved the test. Explicit collate test already ex= ists.

I merged this test into an existing test to avoid adding yet another
query in the file that has many many queries already. Yes, an explicit
collation test is not needed separately.

Thanks!
=C2=A0

While at it I added comments to explain why we aren't performing
en-masse collation assignment on a GraphTable or GraphPathPattern.

Please let me know what you think of the attached patch.
=C2=A0
This LGTM. I reran the tests and all of them are passin= g.
=C2=A0
Thanks,
Satya
--00000000000042b44d064f82a699--