Re: SQL/JSON: functions

On 3/26/21 4:49 PM, Andrew Dunstan wrote:
> On 3/26/21 4:22 PM, Andrew Dunstan wrote:
>> On 3/8/21 1:55 PM, Ibrar Ahmed wrote:
>>> On Sat, Jan 23, 2021 at 3:37 PM Erik Rijkers <er(at)xs4all(dot)nl
>>> <mailto:er(at)xs4all(dot)nl>> wrote:
>>>
>>> On 2021-01-20 03:49, Nikita Glukhov wrote:
>>>
>>> > [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
>>> > [0002-SQL-JSON-constructors-v52.patch.gz]
>>> > [0003-IS-JSON-predicate-v52.patch.gz]
>>> > [0004-SQL-JSON-query-functions-v52.patch.gz]
>>> > [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
>>> > [0006-GUC-sql_json-v52.patch.gz]
>>>
>>> Hi,
>>>
>>> I read through the file func.sgml (only that file) and put the
>>> errors/peculiarities in the attached diff.  (Small stuff; typos
>>> really)
>>>
>>>
>>> Your patch includes a CREATE TABLE my_films + INSERT, to run the
>>> examples against.  I think this is a great idea and we should do
>>> it more
>>> often.
>>>
>>> But, the table has a text-column to contain the subsequently inserted
>>> json values. The insert runs fine but it turns out that some later
>>> examples queries only run against a jsonb column.  So I propose to
>>> change:
>>>    CREATE TABLE my_films (js text);
>>> to:
>>>    CREATE TABLE my_films (js jsonb);
>>>
>>> This change is not yet included in the attached file.  An alternative
>>> would be to cast the text-column in the example queries as js::jsonb
>>>
>>>
>>> I also noticed that some errors were different in the sgml file
>>> than 'in
>>> the event':
>>>
>>>
>>>     SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM
>>> my_films_jsonb;
>>>     (table 'my_films_jsonb' is the same as your 'my_films', but
>>> with js
>>> as a jsonb column)
>>>
>>> manual says: "ERROR: more than one SQL/JSON item"
>>>   in reality: "ERROR: JSON path expression in JSON_QUERY should
>>> return
>>> singleton item without wrapper"
>>>          and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item
>>> sequence into array"
>>>
>>>
>>> Thanks,
>>>
>>> Erik Rijkers
>>>
>>> >
>>> > --
>>> > Nikita Glukhov
>>> > Postgres Professional: http://www.postgrespro.com
>>> <http://www.postgrespro.com>
>>> > The Russian Postgres Company
>>>
>>>
>>> The patch (func.sgml.20210123.diff) does not apply successfully.
>>>
>>> http://cfbot.cputube.org/patch_32_2901.log
>>> <http://cfbot.cputube.org/patch_32_2901.log>
>>>
>>> ----
>>> === Applying patches on top of PostgreSQL commit ID 0ce4cd04da558178b0186057b721c50a00b7a945 ===
>>> === applying patch ./func.sgml.20210123.diff
>>> patching file doc/src/sgml/func.sgml
>>> Hunk #1 FAILED at 16968.
>>> Hunk #2 FAILED at 17034.
>>> ...
>>> Hunk #19 FAILED at 18743.
>>> 19 out of 19 hunks FAILED -- saving rejects to file doc/src/sgml/func.sgml.rej
>>> ----
>>>
>>> Can we get a rebase? 
>>>
>>> I am marking the patch "Waiting on Author".
>>
>>
>> I've rebased this, and applied some of Erik's changes.
>>
>>
>> I'll set it back to 'Needs Review' if the cfbot is happy.
>
> It's not. There are errors  when building with llvm. I'll investigate.

Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
built with LLVM:

cache gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type
-Wformat-security -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -Wno-format-truncation
-Wno-stringop-truncation -g -O2  -fPIC -D__STDC_LIMIT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_GNU_SOURCE
-I/usr/include  -I../../../../src/include
-I/home/andrew/pgl/pg_head/src/include  -D_GNU_SOURCE   -c -o
llvmjit_expr.o /home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c
In file included from /home/andrew/pgl/pg_head/src/include/postgres.h:46,
                 from
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:16:
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c: In
function ‘llvm_compile_expr’:
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:34: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                  ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:34: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                  ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:52: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                                    ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:52: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                                    ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:71:27:
note: in definition of macro ‘build_EvalXFunc’
   71 |         ((LLVMValueRef[]){__VA_ARGS__}))
      |                           ^~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:71:27:
note: in definition of macro ‘build_EvalXFunc’
   71 |         ((LLVMValueRef[]){__VA_ARGS__}))
      |                           ^~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:18:
warning: passing argument 5 of ‘build_EvalXFuncInt’ from incompatible
pointer type [-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                  ^~~~~~~~~~
      |                  |
      |                  LLVMValueRef {aka struct LLVMOpaqueValue *}
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:69:48:
note: in definition of macro ‘build_EvalXFunc’
   69 |  build_EvalXFuncInt(b, mod, funcname, v_state, op, \
      |                                                ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:63:27:
note: expected ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’} but
argument is of type ‘LLVMValueRef’ {aka ‘struct LLVMOpaqueValue *’}
   63 |             ExprEvalStep *op,
      |             ~~~~~~~~~~~~~~^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2349:29:
error: ‘i’ undeclared (first use in this function)
 2349 |     LLVMBuildBr(b, opblocks[i + 1]);
      |                             ^
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2349:29:
note: each undeclared identifier is reported only once for each function
it appears in
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:248:3:
warning: enumeration value ‘EEOP_JSON_CONSTRUCTOR’ not handled in switch
[-Wswitch]
  248 |   switch (opcode)
      |   ^~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:248:3:
warning: enumeration value ‘EEOP_IS_JSON’ not handled in switch [-Wswitch]
make[2]: *** [<builtin>: llvmjit_expr.o] Error 1
make[2]: Leaving directory
'/home/andrew/pgl/pg_head/bfroot/HEAD/pgsql.build/src/backend/jit/llvm'
make[1]: *** [Makefile:42: all-backend/jit/llvm-recurse] Error 2
make[1]: Leaving directory
'/home/andrew/pgl/pg_head/bfroot/HEAD/pgsql.build/src'

There is also a bug that results in a warning in gram.y, but fixing it
doesn't affect this issue. Nikita, please look into this ASAP.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com