Re: sql/json query function JsonBehavior default expression's collation may differ from returning type's collation

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: sql/json query function JsonBehavior default expression's collation may differ from returning type's collation
Date: 2025-10-09 04:24:13
Message-ID: CA+HiwqHAdZoYx=_10N=DfKzxNCMaBAxOa=eAp1jnkxxKPcCiUw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Jian,

On Thu, Oct 9, 2025 at 12:48 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> On Wed, Oct 8, 2025 at 12:10 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >
> > After sleeping on this, I realized the right fix is not to strip
> > CollateExpr in transformJsonBehavior(), but to stop recursing on
> > JsonBehavior.expr in exprSetCollation(). With this patch, the parser
> > now ensures that the JsonBehavior.expr has the correct collation, so
> > the recursion isn’t needed. There is now an Assert in its place that
> > checks that JsonBehavior.expr already has the target collation.
> >
>
> hi, Amit.
> thanks for pushing it.
>
> https://git.postgresql.org/cgit/postgresql.git/commit/?id=ef5e60a9d352a97791af632e0d26a572bc88e921
>
> it took me a little time to understand why
> ``
> exprSetCollation
> case T_JsonBehavior:
> Assert(((JsonBehavior *) expr)->expr == NULL ||
> exprCollation(((JsonBehavior *) expr)->expr) == collation);
> ``
> works, after looking at assign_collations_walker.
>
> it will recursively set JsonBehavior->expr collation first then call
> exprSetCollation for JsonBehavior itself.
>
> I will double-check other JSON constructs with RETURNING clauses for potential
> collation issues in the future.

Yes, that would be great. Thanks for your close attention to this area of code.

--
Thanks, Amit Langote

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2025-10-09 04:27:49 Re: new environment variable INITDB_LOCALE_PROVIDER
Previous Message Tom Lane 2025-10-09 04:20:49 Re: What is the build strategy between make and meson?