Re: remaining sql/json patches - Mailing list pgsql-hackers

On Tue, Apr 2, 2024 at 9:57 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Please let me know if you have further comments on 0001.  I'd like to
> get that in before spending more energy on 0002.
>

hi. some issues with the doc.
i think, some of the "path expression" can be replaced by
"<replaceable>path_expression</replaceable>".
maybe not all of them.

+  <variablelist>
+   <varlistentry>
+    <term>
+     <literal><replaceable>context_item</replaceable>,
<replaceable>path_expression</replaceable> <optional>
<literal>AS</literal> <replaceable>json_path_name</replaceable>
</optional> <optional> <literal>PASSING</literal> {
<replaceable>value</replaceable> <literal>AS</literal>
<replaceable>varname</replaceable> } <optional>,
...</optional></optional></literal>
+    </term>
+    <listitem>
+    <para>
+     The input data to query, the JSON path expression defining the query,
+     and an optional <literal>PASSING</literal> clause, which can provide data
+     values to the <replaceable>path_expression</replaceable>.
+     The result of the input data
+     evaluation is called the <firstterm>row pattern</firstterm>. The row
+     pattern is used as the source for row values in the constructed view.
+    </para>
+    </listitem>
+   </varlistentry>

maybe
change this part "The input data to query, the JSON path expression
defining the query,"
to
`
<replaceable>context_item</replaceable> is the input data to query,
<replaceable>path_expression</replaceable> is the JSON path expression
defining the query,
`

+    <para>
+     Specifying <literal>FORMAT JSON</literal> makes it explcit that you
+     expect that the value to be a valid <type>json</type> object.
+    </para>
"explcit" change to "explicit", or should it be "explicitly"?
also FORMAT JSON can be override by OMIT QUOTES.
SELECT sub.* FROM JSON_TABLE('{"a":{"z1": "a"}}', '$.a' COLUMNS(xx
TEXT format json path '$.z1' omit quotes))sub;
it return not double quoted literal 'a', which cannot be a valid json.

create or replace FUNCTION test_format_json() returns table (thetype
text, is_ok bool) AS $$
declare
    part1_sql text := $sql$SELECT sub.* FROM JSON_TABLE('{"a":{"z1":
"a"}}', '$.a'  COLUMNS(xx $sql$;
    part2_sql text := $sql$ format json path '$.z1' omit quotes))sub $sql$;
    run_status bool := true;
    r record;
    fin record;
BEGIN
    for r in
        select format_type(oid, -1) as aa
        from pg_type where  typtype = 'b' and typarray != 0 and
typnamespace = 11 and typnotnull is false
    loop
        begin
            -- raise notice '%',CONCAT_WS(' ', part1_sql, r.aa, part2_sql);
            -- raise notice 'r.aa %', r.aa;
            run_status := true;
            execute CONCAT_WS(' ', part1_sql, r.aa, part2_sql) into fin;
            return query select r.aa, run_status;
                exception when others then
                begin
                    run_status := false;
                    return query select r.aa, run_status;
                end;
        end;
    end loop;
END;
$$ language plpgsql;
create table sss_1 as select * from test_format_json();
select * from sss_1 where is_ok is true;

use the above query, I've figure out that FORMAT JSON can apply to the
following types:
bytea
name
text
json
bpchar
character varying
jsonb
and these type's customized domain type.

overall, the idea is that:
     Specifying <literal>FORMAT JSON</literal> makes it explicitly that you
     expect that the value to be a valid <type>json</type> object.
    <literal>FORMAT JSON</literal> can be overridden by OMIT QUOTES
specification, which can make the return value not a valid
<type>json</type>.
    <literal>FORMAT JSON</literal> can only work with certain kinds of
data types.
-----------------------------------------------------------------------------------------------
+    <para>
+     Optionally, you can add <literal>ON ERROR</literal> clause to define
+     error behavior.
+    </para>
I think "error behavior" may refer to "what kind of error message it will omit"
but here, it's about what to do when an error happens.
so I guess it's misleading.

maybe we can explain it similar to json_exist.
+    <para>
+     Optionally, you can add <literal>ON ERROR</literal> clause to define
+      the behavior if an error occurs.
+    </para>

+    <para>
+     The specified <parameter>type</parameter> should have a cast from the
+     <type>boolean</type>.
+    </para>
should be
+    <para>
+     The specified <replaceable>type</replaceable> should have a cast from the
+     <type>boolean</type>.
+    </para>


+    <para>
+     Inserts a SQL/JSON value into the output row.
+    </para>
maybe
+    <para>
+     Inserts a value that the data type is
<replaceable>type</replaceable> into the output row.
+    </para>

+    <para>
+     Inserts a boolean item into each output row.
+    </para>
maybe changed to:
+    <para>
+     Inserts a value that the data type is
<replaceable>type</replaceable> into the output row.
+    </para>

"name type EXISTS" branch mentioned: "The specified type should have a
cast from the boolean."
but "name type [FORMAT JSON [ENCODING UTF8]] [ PATH path_expression ]"
never mentioned the "type"parameter.
maybe add one para, something like:
"after apply path_expression, the yield value cannot be coerce to
<replaceable>type</replaceable> it will return null"