Re: remaining sql/json patches

On Fri, Sep 29, 2023 at 1:57 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Sep 28, 2023 at 8:04 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > On 2023-Sep-27, Amit Langote wrote:
> > > Maybe the following is better:
> > >
> > > + /*
> > > + * For expression nodes that support soft errors. Should be set to NULL
> > > + * before calling ExecInitExprRec() if the caller wants errors thrown.
> > > + */
> > >
> > > ...as in the attached.
> >
> > That's good.
> >
> > > Alvaro, do you think your concern regarding escontext not being in the
> > > right spot in the ExprState struct is addressed? It doesn't seem very
> > > critical to me to place it in the struct's 1st cacheline, because
> > > escontext is not accessed in performance critical paths such as during
> > > expression evaluation, especially with the latest version. (It would
> > > get accessed during evaluation with previous versions.)
> > >
> > > If so, I'd like to move ahead with committing it.
> >
> > Yeah, looks OK to me in v21.
>
> Thanks. I will push the attached 0001 shortly.

Pushed this 30 min ago (no email on -committers yet!) and am looking
at the following llvm crash reported by buildfarm animal pogona [1]:

#0 __pthread_kill_implementation (threadid=<optimized out>,
signo=signo(at)entry=6, no_tid=no_tid(at)entry=0) at
./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
#0 __pthread_kill_implementation (threadid=<optimized out>,
signo=signo(at)entry=6, no_tid=no_tid(at)entry=0) at
./nptl/pthread_kill.c:44
#1 0x00007f5bcebcb15f in __pthread_kill_internal (signo=6,
threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2 0x00007f5bceb7d472 in __GI_raise (sig=sig(at)entry=6) at
../sysdeps/posix/raise.c:26
#3 0x00007f5bceb674b2 in __GI_abort () at ./stdlib/abort.c:79
#4 0x00007f5bceb673d5 in __assert_fail_base (fmt=0x7f5bcecdbdc8
"%s%s%s:%u: %s%sAssertion `%s' failed.\\n%n",
assertion=assertion(at)entry=0x7f5bc1336419 "(i >= FTy->getNumParams() ||
FTy->getParamType(i) == Args[i]->getType()) && \\"Calling a function
with a bad signature!\\"", file=file(at)entry=0x7f5bc1336051
"/home/bf/src/llvm-project-5/llvm/lib/IR/Instructions.cpp",
line=line(at)entry=299, function=function(at)entry=0x7f5bc13362af "void
llvm::CallInst::init(llvm::FunctionType *, llvm::Value *,
ArrayRef<llvm::Value *>, ArrayRef<llvm::OperandBundleDef>, const
llvm::Twine &)") at ./assert/assert.c:92
#5 0x00007f5bceb763a2 in __assert_fail (assertion=0x7f5bc1336419 "(i
>= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType())
&& \\"Calling a function with a bad signature!\\"",
file=0x7f5bc1336051
"/home/bf/src/llvm-project-5/llvm/lib/IR/Instructions.cpp", line=299,
function=0x7f5bc13362af "void llvm::CallInst::init(llvm::FunctionType
*, llvm::Value *, ArrayRef<llvm::Value *>,
ArrayRef<llvm::OperandBundleDef>, const llvm::Twine &)") at
./assert/assert.c:101
#6 0x00007f5bc110f138 in llvm::CallInst::init (this=0x557a91f3e508,
FTy=0x557a91ed9ae0, Func=0x557a91f8be88, Args=..., Bundles=...,
NameStr=...) at
/home/bf/src/llvm-project-5/llvm/lib/IR/Instructions.cpp:297
#7 0x00007f5bc0fa579d in llvm::CallInst::CallInst
(this=0x557a91f3e508, Ty=0x557a91ed9ae0, Func=0x557a91f8be88,
Args=..., Bundles=..., NameStr=..., InsertBefore=0x0) at
/home/bf/src/llvm-project-5/llvm/include/llvm/IR/Instructions.h:1934
#8 0x00007f5bc0fa538c in llvm::CallInst::Create (Ty=0x557a91ed9ae0,
Func=0x557a91f8be88, Args=..., Bundles=..., NameStr=...,
InsertBefore=0x0) at
/home/bf/src/llvm-project-5/llvm/include/llvm/IR/Instructions.h:1444
#9 0x00007f5bc0fa51f9 in llvm::IRBuilder<llvm::ConstantFolder,
llvm::IRBuilderDefaultInserter>::CreateCall (this=0x557a91f9c6a0,
FTy=0x557a91ed9ae0, Callee=0x557a91f8be88, Args=..., Name=...,
FPMathTag=0x0) at
/home/bf/src/llvm-project-5/llvm/include/llvm/IR/IRBuilder.h:1669
#10 0x00007f5bc100edda in llvm::IRBuilder<llvm::ConstantFolder,
llvm::IRBuilderDefaultInserter>::CreateCall (this=0x557a91f9c6a0,
Callee=0x557a91f8be88, Args=..., Name=..., FPMathTag=0x0) at
/home/bf/src/llvm-project-5/llvm/include/llvm/IR/IRBuilder.h:1663
#11 0x00007f5bc100714e in LLVMBuildCall (B=0x557a91f9c6a0,
Fn=0x557a91f8be88, Args=0x7ffde6fa0b50, NumArgs=6, Name=0x7f5bc30b648c
"funccall_iocoerce_in_safe") at
/home/bf/src/llvm-project-5/llvm/lib/IR/Core.cpp:2964
#12 0x00007f5bc30af861 in llvm_compile_expr (state=0x557a91fbeac0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/jit/llvm/llvmjit_expr.c:1373
#13 0x0000557a915992db in jit_compile_expr
(state=state(at)entry=0x557a91fbeac0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/jit/jit.c:177
#14 0x0000557a9123071d in ExecReadyExpr
(state=state(at)entry=0x557a91fbeac0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execExpr.c:880
#15 0x0000557a912340d7 in ExecBuildProjectionInfo
(targetList=0x557a91fa6b58, econtext=<optimized out>, slot=<optimized
out>, parent=parent(at)entry=0x557a91f430a8,
inputDesc=inputDesc(at)entry=0x0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execExpr.c:484
#16 0x0000557a9124e61e in ExecAssignProjectionInfo
(planstate=planstate(at)entry=0x557a91f430a8,
inputDesc=inputDesc(at)entry=0x0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execUtils.c:547
#17 0x0000557a91274961 in ExecInitNestLoop
(node=node(at)entry=0x557a91f9e5d8, estate=estate(at)entry=0x557a91f425a0,
eflags=<optimized out>, eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/nodeNestloop.c:308
#18 0x0000557a9124760f in ExecInitNode (node=0x557a91f9e5d8,
estate=estate(at)entry=0x557a91f425a0, eflags=eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execProcnode.c:298
#19 0x0000557a91255d39 in ExecInitAgg (node=node(at)entry=0x557a91f91540,
estate=estate(at)entry=0x557a91f425a0, eflags=eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/nodeAgg.c:3306
#20 0x0000557a912476bf in ExecInitNode (node=0x557a91f91540,
estate=estate(at)entry=0x557a91f425a0, eflags=eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execProcnode.c:341
#21 0x0000557a912770c3 in ExecInitSort
(node=node(at)entry=0x557a91f9e850, estate=estate(at)entry=0x557a91f425a0,
eflags=eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/nodeSort.c:265
#22 0x0000557a91247667 in ExecInitNode
(node=node(at)entry=0x557a91f9e850, estate=estate(at)entry=0x557a91f425a0,
eflags=eflags(at)entry=33) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execProcnode.c:321
#23 0x0000557a912402f5 in InitPlan (eflags=33,
queryDesc=0x557a91fa6fb8) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execMain.c:968
#24 standard_ExecutorStart (queryDesc=queryDesc(at)entry=0x557a91fa6fb8,
eflags=33, eflags(at)entry=1) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execMain.c:266
#25 0x0000557a912403c9 in ExecutorStart
(queryDesc=queryDesc(at)entry=0x557a91fa6fb8, eflags=1) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/executor/execMain.c:145
#26 0x0000557a911c2153 in ExplainOnePlan
(plannedstmt=plannedstmt(at)entry=0x557a91fa6ea8, into=into(at)entry=0x0,
es=es(at)entry=0x557a91f932e8,
queryString=queryString(at)entry=0x557a91dbd650 "EXPLAIN (COSTS
OFF)\\nSELECT DISTINCT (i || '/' || j)::pg_lsn f\\n FROM
generate_series(1, 10) i,\\n generate_series(1, 10) j,\\n
generate_series(1, 5) k\\n WHERE i <= 10 AND j > 0 AND j <= 10\\n
O"..., params=params(at)entry=0x0, queryEnv=queryEnv(at)entry=0x0,
planduration=0x7ffde6fa1258, bufusage=0x0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/commands/explain.c:590
#27 0x0000557a911c23b2 in ExplainOneQuery (query=<optimized out>,
cursorOptions=cursorOptions(at)entry=2048, into=into(at)entry=0x0,
es=es(at)entry=0x557a91f932e8, queryString=0x557a91dbd650 "EXPLAIN (COSTS
OFF)\\nSELECT DISTINCT (i || '/' || j)::pg_lsn f\\n FROM
generate_series(1, 10) i,\\n generate_series(1, 10) j,\\n
generate_series(1, 5) k\\n WHERE i <= 10 AND j > 0 AND j <= 10\\n
O"..., params=params(at)entry=0x0, queryEnv=0x0) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/commands/explain.c:419
#28 0x0000557a911c2ddb in ExplainQuery
(pstate=pstate(at)entry=0x557a91f3eb18, stmt=stmt(at)entry=0x557a91e881d0,
params=params(at)entry=0x0, dest=dest(at)entry=0x557a91f3ea88) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/include/nodes/nodes.h:193
#29 0x0000557a91413811 in standard_ProcessUtility
(pstmt=0x557a91e88280, queryString=0x557a91dbd650 "EXPLAIN (COSTS
OFF)\\nSELECT DISTINCT (i || '/' || j)::pg_lsn f\\n FROM
generate_series(1, 10) i,\\n generate_series(1, 10) j,\\n
generate_series(1, 5) k\\n WHERE i <= 10 AND j > 0 AND j <= 10\\n
O"..., readOnlyTree=<optimized out>, context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x557a91f3ea88, qc=0x7ffde6fa1500) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/utility.c:870
#30 0x0000557a91413ed9 in ProcessUtility
(pstmt=pstmt(at)entry=0x557a91e88280, queryString=<optimized out>,
readOnlyTree=<optimized out>,
context=context(at)entry=PROCESS_UTILITY_TOPLEVEL, params=<optimized
out>, queryEnv=<optimized out>, dest=0x557a91f3ea88,
qc=0x7ffde6fa1500) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/utility.c:530
#31 0x0000557a91411537 in PortalRunUtility
(portal=portal(at)entry=0x557a91e35970, pstmt=0x557a91e88280,
isTopLevel=true, setHoldSnapshot=setHoldSnapshot(at)entry=true,
dest=dest(at)entry=0x557a91f3ea88, qc=qc(at)entry=0x7ffde6fa1500) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/pquery.c:1158
#32 0x0000557a914119a4 in FillPortalStore
(portal=portal(at)entry=0x557a91e35970, isTopLevel=isTopLevel(at)entry=true)
at /home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/include/nodes/nodes.h:193
#33 0x0000557a91411d6d in PortalRun
(portal=portal(at)entry=0x557a91e35970,
count=count(at)entry=9223372036854775807,
isTopLevel=isTopLevel(at)entry=true, run_once=run_once(at)entry=true,
dest=dest(at)entry=0x557a91e88900, altdest=altdest(at)entry=0x557a91e88900,
qc=0x7ffde6fa1700) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/pquery.c:763
#34 0x0000557a9140d65f in exec_simple_query
(query_string=query_string(at)entry=0x557a91dbd650 "EXPLAIN (COSTS
OFF)\\nSELECT DISTINCT (i || '/' || j)::pg_lsn f\\n FROM
generate_series(1, 10) i,\\n generate_series(1, 10) j,\\n
generate_series(1, 5) k\\n WHERE i <= 10 AND j > 0 AND j <= 10\\n
O"...) at /home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/postgres.c:1272
#35 0x0000557a9140e305 in PostgresMain (dbname=<optimized out>,
username=<optimized out>) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/tcop/postgres.c:4652
#36 0x0000557a91372bf0 in BackendRun (port=0x557a91de8730) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/postmaster/postmaster.c:4439
#37 BackendStartup (port=0x557a91de8730) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/postmaster/postmaster.c:4167
#38 ServerLoop () at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/postmaster/postmaster.c:1781
#39 0x0000557a9137488e in PostmasterMain (argc=argc(at)entry=8,
argv=argv(at)entry=0x557a91d7cc10) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/postmaster/postmaster.c:1465
#40 0x0000557a912a001e in main (argc=8, argv=0x557a91d7cc10) at
/home/bf/bf-build/pogona/HEAD/pgsql.build/../pgsql/src/backend/main/main.c:198
$1 = {si_signo = 6, si_errno = 0, si_code = -6, _sifields = {_pad =
{3110875, 1000, 0 <repeats 26 times>}, _kill = {si_pid = 3110875,
si_uid = 1000}, _timer = {si_tid = 3110875, si_overrun = 1000,
si_sigval = {sival_int = 0, sival_ptr = 0x0}}, _rt = {si_pid =
3110875, si_uid = 1000, si_sigval = {sival_int = 0, sival_ptr = 0x0}},
_sigchld = {si_pid = 3110875, si_uid = 1000, si_status = 0, si_utime =
0, si_stime = 0}, _sigfault = {si_addr = 0x3e8002f77db, _addr_lsb = 0,
_addr_bnd = {_lower = 0x0, _upper = 0x0}}, _sigpoll = {si_band =
4294970406875, si_fd = 0}, _sigsys = {_call_addr = 0x3e8002f77db,
_syscall = 0, _arch = 0}}}

This seems to me to be complaining about the following addition:

+ {
+ Oid ioparam = op->d.iocoerce.typioparam;
+ LLVMValueRef v_params[6];
+ LLVMValueRef v_success;
+
+ v_params[0] = l_ptr_const(op->d.iocoerce.finfo_in,
+ l_ptr(StructFmgrInfo));
+ v_params[1] = v_output;
+ v_params[2] = l_oid_const(lc, ioparam);
+ v_params[3] = l_int32_const(lc, -1);
+ v_params[4] = l_ptr_const(op->d.iocoerce.escontext,
+
l_ptr(StructErrorSaveContext));

- LLVMBuildStore(b, v_retval, v_resvaluep);
+ /*
+ * InputFunctionCallSafe() will write directly into
+ * *op->resvalue.
+ */
+ v_params[5] = v_resvaluep;
+
+ v_success = LLVMBuildCall(b, llvm_pg_func(mod,
"InputFunctionCallSafe"),
+ v_params, lengthof(v_params),
+ "funccall_iocoerce_in_safe");
+
+ /*
+ * Return null if InputFunctionCallSafe() encountered
+ * an error.
+ */
+ v_resnullp = LLVMBuildICmp(b, LLVMIntEQ, v_success,
+ l_sbool_const(0), "");
+ }

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2023-10-02%2003%3A50%3A20