From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Erik Rijkers <er(at)xs4all(dot)nl>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, jian he <jian(dot)universality(at)gmail(dot)com> |
Subject: | Re: remaining sql/json patches |
Date: | 2023-07-21 10:33:11 |
Message-ID: | CA+HiwqFPxddm_sG-fZZESVi9FRsR79TPKjvoFXWZ9T=x=L6afw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Alvaro,
Thanks for taking a look.
On Fri, Jul 21, 2023 at 1:02 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2023-Jul-21, Amit Langote wrote:
>
> > I’m thinking of pushing 0001 and 0002 tomorrow barring objections.
>
> 0001 looks reasonable to me. I think you asked whether to squash that
> one with the other bugfix commit for the same code that you already
> pushed to master; I think there's no point in committing as separate
> patches, because the first one won't show up in the git_changelog output
> as a single entity with the one in 16, so it'll just be additional
> noise.
OK, pushed 0001 to HEAD and b6e1157e7d + 0001 to 16.
> I've looked at 0002 at various points in time and I think it looks
> generally reasonable. I think your removal of a couple of newlines
> (where originally two appear in sequence) is unwarranted; that the name
> to_json[b]_worker is ugly for exported functions (maybe "datum_to_json"
> would be better, or you may have better ideas);
Went with datum_to_json[b]. Created a separate refactoring patch for
this, attached as 0001.
Created another refactoring patch for the hunks related to renaming of
a nonterminal in gram.y, attached as 0002.
> and that the omission of
> the stock comment in the new stanzas in FigureColnameInternal() is
> strange.
Yes, fixed.
> But I don't have anything serious. Do add some ecpg tests ...
Added.
> Also, remember to pgindent and bump catversion, if you haven't already.
Will do. Wasn't sure myself whether the catversion should be bumped,
but I suppose it must be because ruleutils.c has changed.
Attaching latest patches. Will push 0001, 0002, and 0003 on Monday to
avoid worrying about the buildfarm on a Friday evening.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v10-0002-Rename-a-nonterminal-used-in-SQL-JSON-grammar.patch | application/octet-stream | 3.3 KB |
v10-0006-Claim-SQL-standard-compliance-for-SQL-JSON-featu.patch | application/octet-stream | 2.3 KB |
v10-0003-Add-more-SQL-JSON-constructor-functions.patch | application/octet-stream | 71.7 KB |
v10-0005-JSON_TABLE.patch | application/octet-stream | 164.2 KB |
v10-0004-SQL-JSON-query-functions.patch | application/octet-stream | 209.4 KB |
v10-0001-Some-refactoring-to-export-json-b-conversion-fun.patch | application/octet-stream | 11.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiro Ikeda | 2023-07-21 10:35:36 | Re: Support worker_spi to execute the function dynamically. |
Previous Message | Richard Guo | 2023-07-21 10:02:10 | Assert failure on bms_equal(child_joinrel->relids, child_joinrelids) |