On Mon, Oct 6, 2025 at 10:58 PM Amit Langote <amitlangote09@gmail.com> wrote:
> (Sorry for the long delay -- I meant to get back to this a while ago.)
>
> Thanks for posting v2 of the patch. I’ve made a few follow-up changes
> (v3 attached):
>
> * Moved the regression tests from sqljson_queryfuncs.sql to
> collation.icu.utf8.sql to avoid failures on buildfarm machines without
> ICU support.
>
> * Adjusted the collation-mismatch check in transformJsonBehavior() so
> that it runs last within the DEFAULT-handling block. That keeps the
> control flow cleaner and avoids affecting existing tests that already
> fail earlier checks, preventing unnecessary regression output churn.
>
> * Did a few cosmetic edits and fixed the error code and message text.
>
> Otherwise, behavior and coverage remain the same.
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.
Attached v4.
--
Thanks, Amit Langote