| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
| Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: transformJsonFuncExpr pathspec cache lookup failed |
| Date: | 2025-11-27 03:16:22 |
| Message-ID: | CA+HiwqHkPtoH_9k+QZ7oAm1EYO9GxWRD5FcHqdGUK-xKpKxshA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Nov 26, 2025 at 8:57 PM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
> On Wed, 26 Nov 2025 at 10:32, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Wed, Nov 19, 2025 at 11:55 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> > > On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
> > > >
> > > > Hi!
> > > > I tried your fix and this indeed fixes an issue. Two minor comments:
> > > >
> > > > First,
> > > > in the `src/backend/parser/parse_expr.c` fil there are multiple
> > > > examples of working with `coerce_to_target_type`, they all share
> > > > different coding practice:
> > > >
> > > > ```
> > > > coerced_expr = coerce_to_target_type(.., expr, ..)
> > > >
> > > > if (coerced_expr == NULL)
> > > > ereport();
> > > >
> > > >
> > > > expr = coerced_expr;
> > > > ```
> > > >
> > > > Instead of
> > > > ```
> > > > expr = coerce_to_target_type(.., expr, ..)
> > > >
> > > > if (expr == NULL)
> > > > ereport();
> > > > ```
> > > >
> > > > Let's be consistent?
> > > >
> > >
> > > IMHO,
> > > > coerced_expr = coerce_to_target_type(.., expr, ..)
> > > is better than
> > > > expr = coerce_to_target_type(.., expr, ..)
> > >
> > > I changed accordingly.
> >
> > I agree, though I prefer a different name for that coerced_* variable.
> > Also, let’s define the _type and _loc variables inside the error
> > block. Updated patch attached.
> >
> > Please feel free to post a patch for the 2nd issue.
>
> v2 lgtm
Pushed after moving the _type, _loc variable declarations outside
after all, because I had missed that they are passed to
coerce_to_target_type() as well in Jian's patch.
--
Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Pavel Stehule | 2025-11-27 03:18:40 | Re: [PATCH] contrib: Add pg_datemath extension with datediff function |
| Previous Message | Amit Langote | 2025-11-27 03:10:54 | Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp |