Subject: Re: SQL/JSON revisited

From: Erik Rijkers <[email-masked]>
Date: Tue, 28 Mar 2023 15:40:24 +0200

Op 3/27/23 om 20:54 schreef Alvaro Herrera:
> Docs amended as I threatened.  Other than that, this has required more

 > [v12-0001-SQL-JSON-constructors.patch]
 > [v12-0001-delta-uniqueifyJsonbObject-bugfix.patch]

In doc/src/sgml/func.sgml, some minor stuff:

'which specify the data type returned'  should be
'which specifies the data type returned'

In the json_arrayagg() description, it says:
'If ABSENT ON NULL is specified, any NULL values are omitted.'
That's true, but as omitting NULL values is the default (i.e., also 
without that clause) maybe it's better to say:
'Any NULL values are omitted unless NULL ON NULL is specified'

I've found no bugs in functionality.

Thanks,

Erik Rijkers

---

From: Alvaro Herrera <[email-masked]>
Date: Wed, 29 Mar 2023 12:27:58 +0200

On 2023-Mar-28, Erik Rijkers wrote:

> That's true, but as omitting NULL values is the default (i.e., also without
> that clause) maybe it's better to say:

Doh, somehow I misread your report and modified the json_object()
documentation instead after experimenting with it (so now the
ABSENT/NULL ON NULL clause is inconsistenly described everywhere).
Would you mind submitting a patch fixing this mistake?

... and pushed it now, after some more meddling.

I'll rebase the rest of the series now.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Entristecido, Wutra                     (canción de Las Barreras)
echa a Freyr a rodar
y a nosotros al mar"

---

From: Alexander Lakhin <[email-masked]>
Date: Wed, 29 Mar 2023 21:00:00 +0300

Hi,

> I'll rebase the rest of the series now.

Please look at the several minor issues/inconsistencies,
I've spotted in the commit:

1) s/JSON_ARRRAYAGG/JSON_ARRAYAGG/

2)
check_key_uniqueness vs check_unique
IIUC, these are different names of the same entity.

3)
elog(ERROR, "invalid JsonConstructorExprType %d", ctor->type);
vs
elog(ERROR, "invalid JsonConstructorExpr type %d", ctor->type);
I'd choose the latter spelling as the JsonConstructorExprType entity does not exist.

4)
In the block:
     else
     {
         res = (Datum) 0;
         elog(ERROR, "invalid JsonConstructorExpr type %d", ctor->type);
     }
res is assigned but never used.

5)
(expr [FORMAT json_format]) ->? (expr [FORMAT JsonFormat])
(json_format not found anywhere else)

Best regards,
Alexander