For 0001-Common-SQL-JSON-clauses-v51.patch : + /* | implementation_defined_JSON_representation_option (BSON, AVRO etc) */
I don't find implementation_defined_JSON_representation_option in the patchset. Maybe rephrase the above as a comment without implementation_defined_JSON_representation_option ? For getJsonEncodingConst(), should the method error out for the default case of switch (encoding) ? 0002-SQL-JSON-constructors-v51.patch : + Assert(!OidIsValid(collation)); /* result is always an json[b] type */ an json -> a json + /* XXX TEXTOID is default by standard */ + returning->typid = JSONOID; Comment doesn't seem to match the assignment. For json_object_agg_transfn : + if (out->len > 2) + appendStringInfoString(out, ", "); Why length needs to be at least 3 (instead of 2) ? Cheers On Fri, Dec 25, 2020 at 12:26 PM Nikita Glukhov <n.glu...@postgrespro.ru> wrote: > On 17.09.2020 08:41, Michael Paquier wrote: > > On Sat, Jul 18, 2020 at 09:24:11AM -0400, Andrew Dunstan wrote: > > I think patches 5 and 6 need to be submitted to the next commitfest, > This is far too much scope creep to be snuck in under the current CF item. > > I'll look at patches 1-4. > > Even with that, the patch set has been waiting on author for the last > six weeks, so I am marking it as RwF for now. Please feel free to > resubmit. > > Attached 51st version of the patches rebased onto current master. > > > There were some shift/reduce conflicts in SQL grammar that have appeared > after "expr AS keyword" refactoring in 06a7c3154f. I'm not sure if I resolved > them correctly. JSON TEXT pseudotype, introduced in #0006, caused a lot of > grammar conflicts, so it was replaced with simple explicit pg_catalog.json. > > Also new CoercionForm COERCE_SQL_SYNTAX was introduced, and this reminds > custom > function formats that I have used in earlier version of the patches for > deparsing of SQL/JSON constructor expressions that were based on raw json[b] > function calls. These custom function formats were replaced in v43 with > dedicated executor nodes for SQL/JSON constructors. So, I'm not sure is it > worth to try to replace back nodes with new COERCE_SQL_SYNTAX. > > -- > Nikita Glukhov > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > >
