MIME-Version: 1.0
References: 
 <CAHg+QDc4aaiufYSgrwMMPMMRTPtQ66SghcrPFbWJFZMqNaG+BA@mail.gmail.com>
 <CAExHW5s5Q4U3GPHaGJMAZBb8=7bm4pDc9_MWi5AsTaFC-OB2HQ@mail.gmail.com>
 <CAHg+QDfdgVT0qZv6fGcxS7Sk1se8yYBsaVUGoQqbLVVs9PsxZQ@mail.gmail.com>
In-Reply-To: 
 <CAHg+QDfdgVT0qZv6fGcxS7Sk1se8yYBsaVUGoQqbLVVs9PsxZQ@mail.gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Wed, 15 Apr 2026 20:37:18 +0530
Message-ID: 
 <CAExHW5somY+srBAAvYH4-ty6sKSjddQL3DYw_Ah_UhT75ZO_rg@mail.gmail.com>
Subject: Re: Bug: Missing collation assignment for GRAPH_TABLE COLUMNS
 expressions
To: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Peter Eisentraut <peter@eisentraut.org>
Content-Type: multipart/mixed; boundary="0000000000002cd741064f811336"
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5somY%2BsrBAAvYH4-ty6sKSjddQL3DYw_Ah_UhT75ZO_rg%40mail.gmail.com>
Precedence: bulk

--0000000000002cd741064f811336
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 10, 2026 at 11:36=E2=80=AFPM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:
>
> Hi Ashutosh,
>
> On Fri, Apr 10, 2026 at 9:25=E2=80=AFAM Ashutosh Bapat <ashutosh.bapat.os=
s@gmail.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-dependent funct=
ions or operators fail with:
>> >
>> >   ERROR: could not determine which collation to use for upper() functi=
on
>> >   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 sr=
c_upper));
>> > ERROR:  could not determine which collation to use for upper() functio=
n
>> > HINT:  Use the COLLATE clause to set the collation explicitly.
>> >
>> >
>> > In transformRangeGraphTable(), the COLUMNS transformation loop calls t=
ransformExpr()
>> > on each column expression but omits the subsequent assign_expr_collati=
ons() call.  Both
>> > WHERE clause transformation sites in parse_graphtable.c correctly incl=
ude 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.

>>
>>
>>
>> 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 exist=
s.

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.

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.


--
Best Wishes,
Ashutosh Bapat

--0000000000002cd741064f811336
Content-Type: text/x-patch; charset="US-ASCII";
	name="v20260415-0001-Collation-of-expressions-in-GRAPH_TABLE-CO.patch"
Content-Disposition: attachment;
	filename="v20260415-0001-Collation-of-expressions-in-GRAPH_TABLE-CO.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo06p36q0>
X-Attachment-Id: f_mo06p36q0

RnJvbSA3NDgxZjBkMjk1MDhmYzY1NmVmZGQ1ZmYyYTY5MmVlZWE0MDZhYzdhIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBc2h1dG9zaCBCYXBhdCA8YXNodXRvc2guYmFwYXQub3NzQGdt
YWlsLmNvbT4KRGF0ZTogV2VkLCAxNSBBcHIgMjAyNiAxNjoxODozNyArMDUzMApTdWJqZWN0OiBb
UEFUQ0ggdjIwMjYwNDE1IDEvM10gQ29sbGF0aW9uIG9mIGV4cHJlc3Npb25zIGluIEdSQVBIX1RB
QkxFIENPTFVNTlMKIGNsYXVzZQoKR1JBUEhfVEFCTEUgY2xhdXNlIGlzIGNvbnZlcnRlZCBpbnRv
IGEgcmFuZ2V0YWJsZSBlbnRyeSB3aGljaCBpcyBpZ25vcmVkCmJ5IGFzc2lnbl9xdWVyeV9jb2xs
YXRpb25zKCkuIEhlbmNlIHdlIGFzc2lnbiBjb2xsYXRpb25zIHdoaWxlCnRyYW5zZm9ybWluZyBp
dHMgcGFydHMuIEV4cHJlc3Npb25zIGluIENPTFVNTlMgY2xhdXNlIG1pc3NlZCB0aGF0CnRyZWF0
bWVudC4KCldoaWxlIGF0IGFsc28gYWRkIGNvbW1lbnRzIGFib3V0IGNvbGxhdGlvbiBhc3NpZ25t
ZW50IHRvIHRoZSBwYXJ0cyBvZgpHUkFQSF9UQUJMRSBjbGF1c2UgYW5kIGFsc28gZml4IGEgc21h
bGwgZ3JhbW1hciBpc3N1ZS4KClJlcG9ydGVkLWJ5OiBTYXR5YW5hcmF5YW5hIE5hcmxhcHVyYW0g
PHNhdHlhbmFybGFwdXJhbUBnbWFpbC5jb20+CkF1dGhvcjogU2F0eWFuYXJheWFuYSBOYXJsYXB1
cmFtIDxzYXR5YW5hcmxhcHVyYW1AZ21haWwuY29tPgpBdXRob3I6IEFzaHV0b3NoIEJhcGF0IDxh
c2h1dG9zaC5iYXBhdC5vc3NAZ21haWwuY29tPgpEaXNjdXNzaW9uOiBodHRwczovL3d3dy5wb3N0
Z3Jlc3FsLm9yZy9tZXNzYWdlLWlkL0NBSGcrUURjNGFhaXVmWVNncndNTVBNTVJUUHRRNjZTZ2hj
clBGYldKRlpNcU5hRytCQUBtYWlsLmdtYWlsLmNvbQotLS0KIHNyYy9iYWNrZW5kL3BhcnNlci9w
YXJzZV9jbGF1c2UuYyAgICAgICAgIHwgIDYgKysrKysrCiBzcmMvYmFja2VuZC9wYXJzZXIvcGFy
c2VfZ3JhcGh0YWJsZS5jICAgICB8IDEyICsrKysrKysrKysrLQogc3JjL3Rlc3QvcmVncmVzcy9l
eHBlY3RlZC9ncmFwaF90YWJsZS5vdXQgfCAgOCArKysrLS0tLQogc3JjL3Rlc3QvcmVncmVzcy9z
cWwvZ3JhcGhfdGFibGUuc3FsICAgICAgfCAgNCArKy0tCiA0IGZpbGVzIGNoYW5nZWQsIDIzIGlu
c2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvcGFy
c2VyL3BhcnNlX2NsYXVzZS5jIGIvc3JjL2JhY2tlbmQvcGFyc2VyL3BhcnNlX2NsYXVzZS5jCmlu
ZGV4IDk2N2VlYTQ0ZjFjLi40MjcwYzIzODJjNCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcGFy
c2VyL3BhcnNlX2NsYXVzZS5jCisrKyBiL3NyYy9iYWNrZW5kL3BhcnNlci9wYXJzZV9jbGF1c2Uu
YwpAQCAtMTAwMyw2ICsxMDAzLDEyIEBAIHRyYW5zZm9ybVJhbmdlR3JhcGhUYWJsZShQYXJzZVN0
YXRlICpwc3RhdGUsIFJhbmdlR3JhcGhUYWJsZSAqcmd0KQogCQljb2x1bW5zID0gbGFwcGVuZChj
b2x1bW5zLCB0ZSk7CiAJfQogCisJLyoKKwkgKiBBc3NpZ24gY29sbGF0aW9ucyB0byBjb2x1bW4g
ZXhwcmVzc2lvbnMgbm93IHNpbmNlCisJICogYXNzaWduX3F1ZXJ5X2NvbGxhdGlvbnMoKSBkb2Vz
IG5vdCBwcm9jZXNzIHJhbmdldGFibGUgZW50cmllcy4KKwkgKi8KKwlhc3NpZ25fbGlzdF9jb2xs
YXRpb25zKHBzdGF0ZSwgY29sdW1ucyk7CisKIAl0YWJsZV9jbG9zZShyZWwsIE5vTG9jayk7CiAK
IAlwc3RhdGUtPnBfZ3JhcGhfdGFibGVfcHN0YXRlID0gTlVMTDsKZGlmZiAtLWdpdCBhL3NyYy9i
YWNrZW5kL3BhcnNlci9wYXJzZV9ncmFwaHRhYmxlLmMgYi9zcmMvYmFja2VuZC9wYXJzZXIvcGFy
c2VfZ3JhcGh0YWJsZS5jCmluZGV4IDMwZGRjZTVhYTlmLi44NzM4Njg1OWE1YyAxMDA2NDQKLS0t
IGEvc3JjL2JhY2tlbmQvcGFyc2VyL3BhcnNlX2dyYXBodGFibGUuYworKysgYi9zcmMvYmFja2Vu
ZC9wYXJzZXIvcGFyc2VfZ3JhcGh0YWJsZS5jCkBAIC0yNTIsNiArMjUyLDExIEBAIHRyYW5zZm9y
bUdyYXBoRWxlbWVudFBhdHRlcm4oUGFyc2VTdGF0ZSAqcHN0YXRlLCBHcmFwaEVsZW1lbnRQYXR0
ZXJuICpnZXApCiAJZ2VwLT5sYWJlbGV4cHIgPSB0cmFuc2Zvcm1MYWJlbEV4cHIoZ3BzdGF0ZSwg
Z2VwLT5sYWJlbGV4cHIpOwogCiAJZ2VwLT53aGVyZUNsYXVzZSA9IHRyYW5zZm9ybUV4cHIocHN0
YXRlLCBnZXAtPndoZXJlQ2xhdXNlLCBFWFBSX0tJTkRfV0hFUkUpOworCisJLyoKKwkgKiBBc3Np
Z24gY29sbGF0aW9ucyBoZXJlIGZvciB0aGUgcmVhc29uIG1lbnRpb25lZCBpbiB0aGUgcHJvbG9n
dWUgb2YKKwkgKiB0cmFuc2Zvcm1HcmFwaFBhdHRlcm4oKS4KKwkgKi8KIAlhc3NpZ25fZXhwcl9j
b2xsYXRpb25zKHBzdGF0ZSwgZ2VwLT53aGVyZUNsYXVzZSk7CiAKIAlncHN0YXRlLT5jdXJfZ2Vw
ID0gTlVMTDsKQEAgLTM2Niw5ICszNzEsMTQgQEAgdHJhbnNmb3JtUGF0aFBhdHRlcm5MaXN0KFBh
cnNlU3RhdGUgKnBzdGF0ZSwgTGlzdCAqcGF0aF9wYXR0ZXJuKQogICogVHJhbnNmb3JtIGEgR3Jh
cGhQYXR0ZXJuLgogICoKICAqIEEgR3JhcGhQYXR0ZXJuIGNvbnNpc3RzIG9mIGEgbGlzdCBvZiBv
bmUgb3IgbW9yZSBwYXRoIHBhdHRlcm5zIGFuZCBhbgotICogb3B0aW9uYWwgd2hlcmUgY2xhdXNl
LiBUcmFuc2Zvcm0gdGhlbS4gV2UgdXNlIHRoZSBwcmV2aW91c2x5IGNvbnN0cnVjdHVyZQorICog
b3B0aW9uYWwgd2hlcmUgY2xhdXNlLiBUcmFuc2Zvcm0gdGhlbS4gV2UgdXNlIHRoZSBwcmV2aW91
c2x5IGNvbnN0cnVjdGVkCiAgKiBsaXN0IG9mIHZhcmlhYmxlcyBpbiB0aGUgR3JhcGhUYWJsZVBh
cnNlU3RhdGUgdG8gcmVzb2x2ZSBwcm9wZXJ0eSByZWZlcmVuY2VzCiAgKiBpbiB0aGUgV0hFUkUg
Y2xhdXNlLgorICoKKyAqIFNpbmNlIG1vc3QgcGFydHMgb2YgdGhlIEdyYXBoUGF0dGVybiBkbyBu
b3QgcmVxdWlyZSBjb2xsYXRpb24gYXNzaWdubWVudCwgd2UKKyAqIGFzc2lnbiBjb2xsYXRpb25z
IHRvIHRoZSByZXF1aXJlZCBleHByZXNzaW9ucyBhcyB0aGV5IGFyZSB0cmFuc2Zvcm1lZC4gIFRo
aXMKKyAqIGF2b2lkcyB0aGUgbmVlZCB0byB0cmF2ZXJzZSB0aGUgd2hvbGUgR3JhcGhQYXR0ZXJu
IGFnYWluIGFuZCBhdm9pZHMgZXhwb3NpbmcKKyAqIGl0IHRvIGFzc2lnbl9leHByX2NvbGxhdGlv
bnMoKS4KICAqLwogTm9kZSAqCiB0cmFuc2Zvcm1HcmFwaFBhdHRlcm4oUGFyc2VTdGF0ZSAqcHN0
YXRlLCBHcmFwaFBhdHRlcm4gKmdyYXBoX3BhdHRlcm4pCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9y
ZWdyZXNzL2V4cGVjdGVkL2dyYXBoX3RhYmxlLm91dCBiL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0
ZWQvZ3JhcGhfdGFibGUub3V0CmluZGV4IGI1NzllM2RmNjM1Li4wNTdmMjgzYzQzZCAxMDA2NDQK
LS0tIGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9ncmFwaF90YWJsZS5vdXQKKysrIGIvc3Jj
L3Rlc3QvcmVncmVzcy9leHBlY3RlZC9ncmFwaF90YWJsZS5vdXQKQEAgLTY1MiwxMyArNjUyLDEz
IEBAIFNFTEVDVCAqIEZST00gR1JBUEhfVEFCTEUgKGcxIE1BVENIIChhKS0oYSkgQ09MVU1OUyAo
YS52bmFtZSBBUyBzZWxmKSk7CiAgdjMzCiAoMSByb3cpCiAKLS0tIHRlc3QgY29sbGF0aW9uIHNw
ZWNpZmllZCBpbiB0aGUgZXhwcmVzc2lvbgorLS0gdGVzdCBleHBsaWNpdCBhbmQgaW1wbGljaXQg
Y29sbGF0aW9uIGFzc2lnbm1lbnQKIElOU0VSVCBJTlRPIGUzXzMgVkFMVUVTICgyMDAzLCAyMDAz
LCAnRTMzMScsIDEwMDExKTsKLVNFTEVDVCAqIEZST00gR1JBUEhfVEFCTEUgKGcxIE1BVENIIChh
KS1bYl0tPihhKS1bYl0tPihhKSBDT0xVTU5TIChhLnZuYW1lIEFTIHNlbGYsIGIuZW5hbWUgQVMg
bG9vcF9uYW1lKSkgT1JERVIgQlkgbG9vcF9uYW1lIENPTExBVEUgIkMiIEFTQzsKK1NFTEVDVCAq
IEZST00gR1JBUEhfVEFCTEUgKGcxIE1BVENIIChhKS1bYl0tPihhKS1bYl0tPihhKSBDT0xVTU5T
ICh1cHBlcihhLnZuYW1lKSBBUyBzZWxmLCBiLmVuYW1lIEFTIGxvb3BfbmFtZSkpIE9SREVSIEJZ
IGxvb3BfbmFtZSBDT0xMQVRFICJDIiBBU0M7CiAgc2VsZiB8IGxvb3BfbmFtZSAKIC0tLS0tLSst
LS0tLS0tLS0tLQotIHYzMyAgfCBFMzMxCi0gdjMzICB8IGUzMzEKKyBWMzMgIHwgRTMzMQorIFYz
MyAgfCBlMzMxCiAoMiByb3dzKQogCiBTRUxFQ1QgKiBGUk9NIEdSQVBIX1RBQkxFIChnMSBNQVRD
SCAoYSktW2IgSVMgZWwyIFdIRVJFIGIuZW5hbWUgPiAnRTMzMScgQ09MTEFURSAiQyJdLT4oYSkt
W2JdLT4oYSkgQ09MVU1OUyAoYS52bmFtZSBBUyBzZWxmLCBiLmVuYW1lIEFTIGxvb3BfbmFtZSkp
OwpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9zcWwvZ3JhcGhfdGFibGUuc3FsIGIvc3Jj
L3Rlc3QvcmVncmVzcy9zcWwvZ3JhcGhfdGFibGUuc3FsCmluZGV4IDRmZjk4ODE3NDIwLi4yNzgw
NjQ4MThmZiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVncmVzcy9zcWwvZ3JhcGhfdGFibGUuc3Fs
CisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL2dyYXBoX3RhYmxlLnNxbApAQCAtMzk0LDkgKzM5
NCw5IEBAIFNFTEVDVCAqIEZST00gR1JBUEhfVEFCTEUgKGcxIE1BVENIIChhKS1bYl0tPihjKS1b
Yl0tPihkKSBDT0xVTU5TIChhLnZuYW1lIEFTIGFuCiBTRUxFQ1QgKiBGUk9NIEdSQVBIX1RBQkxF
IChnMSBNQVRDSCAoYSktW2NdLShhKSBDT0xVTU5TIChhLnZuYW1lIEFTIHNlbGYsIGMuZW5hbWUg
QVMgbG9vcF9uYW1lKSk7CiBTRUxFQ1QgKiBGUk9NIEdSQVBIX1RBQkxFIChnMSBNQVRDSCAoYSkt
KGEpIENPTFVNTlMgKGEudm5hbWUgQVMgc2VsZikpOwogCi0tLSB0ZXN0IGNvbGxhdGlvbiBzcGVj
aWZpZWQgaW4gdGhlIGV4cHJlc3Npb24KKy0tIHRlc3QgZXhwbGljaXQgYW5kIGltcGxpY2l0IGNv
bGxhdGlvbiBhc3NpZ25tZW50CiBJTlNFUlQgSU5UTyBlM18zIFZBTFVFUyAoMjAwMywgMjAwMywg
J0UzMzEnLCAxMDAxMSk7Ci1TRUxFQ1QgKiBGUk9NIEdSQVBIX1RBQkxFIChnMSBNQVRDSCAoYSkt
W2JdLT4oYSktW2JdLT4oYSkgQ09MVU1OUyAoYS52bmFtZSBBUyBzZWxmLCBiLmVuYW1lIEFTIGxv
b3BfbmFtZSkpIE9SREVSIEJZIGxvb3BfbmFtZSBDT0xMQVRFICJDIiBBU0M7CitTRUxFQ1QgKiBG
Uk9NIEdSQVBIX1RBQkxFIChnMSBNQVRDSCAoYSktW2JdLT4oYSktW2JdLT4oYSkgQ09MVU1OUyAo
dXBwZXIoYS52bmFtZSkgQVMgc2VsZiwgYi5lbmFtZSBBUyBsb29wX25hbWUpKSBPUkRFUiBCWSBs
b29wX25hbWUgQ09MTEFURSAiQyIgQVNDOwogU0VMRUNUICogRlJPTSBHUkFQSF9UQUJMRSAoZzEg
TUFUQ0ggKGEpLVtiIElTIGVsMiBXSEVSRSBiLmVuYW1lID4gJ0UzMzEnIENPTExBVEUgIkMiXS0+
KGEpLVtiXS0+KGEpIENPTFVNTlMgKGEudm5hbWUgQVMgc2VsZiwgYi5lbmFtZSBBUyBsb29wX25h
bWUpKTsKIFNFTEVDVCAqIEZST00gR1JBUEhfVEFCTEUgKGcxIE1BVENIIChhKS1bYl0tPihhKS1b
Yl0tPihhKSBXSEVSRSBiLmVuYW1lID4gJ0UzMzEnIENPTExBVEUgIkMiIENPTFVNTlMgKGEudm5h
bWUgQVMgc2VsZiwgYi5lbmFtZSBBUyBsb29wX25hbWUpKTsKIFNFTEVDVCAqIEZST00gR1JBUEhf
VEFCTEUgKGcxIE1BVENIIChhKS1bYl0tPihhKS1bYl0tPihhKSBDT0xVTU5TIChhLnZuYW1lIEFT
IHNlbGYsIGIuZW5hbWUgQVMgbG9vcF9uYW1lKSkgV0hFUkUgbG9vcF9uYW1lID4gJ0UzMzEnIENP
TExBVEUgIkMiOwoKYmFzZS1jb21taXQ6IGYzMGQwYzcyMGYyZWM5NzlhYjFiNWI0NGIxZjlmMjAx
ZDZlZmRmOGMKLS0gCjIuMzQuMQoK
--0000000000002cd741064f811336--