pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18722: Processing arrays with plpgsql raises errors
8+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2024-11-25 5b8728cd7f 97be02ad00 4aba56adcd 80cd33bad1 a1168855e0 48a6cd1ae6  Fix NULLIF()'s handling of read-write expanded objects.
* BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-23 10:00  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 8+ messages in thread

From: PG Bug reporting form @ 2024-11-23 10:00 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com

The following bug has been logged on the website:

Bug reference:      18722
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 17.2
Operating system:   Ubuntu 22.04
Description:        

The following script:
CREATE FUNCTION make_ia() RETURNS int[] LANGUAGE plpgsql AS
  'declare x int[]; begin x := array[0]; return x; end';
CREATE FUNCTION ia_eq(int[], int[]) RETURNS boolean LANGUAGE plpgsql AS
'begin return array_eq($1, $2); end'; 
CREATE OPERATOR = (procedure = ia_eq, leftarg = int[], rightarg = int[]);
SELECT NULLIF(make_ia(), array[1]::int[]);

fails with:
ERROR:  cache lookup failed for type 2139062143

Also,
SELECT NULLIF(make_ia(), array[1]::int[]) = NULL;

fails with:
ERROR:  invalid memory alloc request size 18446744073642179576

The backtrace of the latter error is:
...
#6  0x000055b75ff0a96e in MemoryContextSizeFailure (context=0x55b760787d70,
size=18446744073642179576, flags=0) at mcxt.c:1170
#7  0x000055b75fefb4ec in MemoryContextCheckSize (context=0x55b760787d70,
size=18446744073642179576, flags=0) at
../../../../src/include/utils/memutils_internal.h:172
#8  0x000055b75fefc087 in AllocSetAllocLarge (context=0x55b760787d70,
size=18446744073642179576, flags=0) at aset.c:705
#9  0x000055b75fefc6c0 in AllocSetAlloc (context=0x55b760787d70,
size=18446744073642179576, flags=0) at aset.c:986
#10 0x000055b75ff0aa40 in MemoryContextAlloc (context=0x55b760787d70,
size=18446744073642179576) at mcxt.c:1200
#11 0x000055b75fd04a64 in copy_byval_expanded_array (eah=0x55b760787e70,
oldeah=0x55b760787e70) at array_expanded.c:197
#12 0x000055b75fd047d7 in expand_array (arraydatum=94246085885576,
parentcontext=0x55b7607b0cd0, metacache=0x7ffd548c2cc0) at
array_expanded.c:106
#13 0x00007f0094e5888f in plpgsql_exec_function (func=0x55b7606f2fb0,
fcinfo=0x55b7607992a8, simple_eval_estate=0x0, simple_eval_resowner=0x0,
procedure_resowner=0x0, atomic=true) at pl_exec.c:564
#14 0x00007f0094e75728 in plpgsql_call_handler (fcinfo=0x55b7607992a8) at
pl_handler.c:276
#15 0x000055b75f9dcd47 in ExecInterpExpr (state=0x55b760799150,
econtext=0x55b760798ef8, isnull=0x7ffd548c32df) at execExprInterp.c:746
...

(gdb) f 12
#12 0x000055b75fd047d7 in expand_array (arraydatum=94246085885576,
parentcontext=0x55b7607b0cd0, metacache=0x7ffd548c2cc0) at
array_expanded.c:106
106                             copy_byval_expanded_array(eah, oldeah);
(gdb) p/x *oldeah
$1 = {hdr = {vl_len_ = 0xffffffff, eoh_methods = 0x55b7602d8f80, eoh_context
= 0x55b760787d70, eoh_rw_ptr = {0x1, 0x3, 0x70, 0x7e, 0x78, 0x60, 0xb7,
0x55, 0x0, 0x0}, eoh_ro_ptr = {0x1, 0x2, 0x70, 0x7e, 0x78, 0x60, 0xb7, 0x55,
0x0, 
      0x0}}, ea_magic = 0x29170a59, ndims = 0x7f7f7f7f, dims =
0x7f7f7f7f7f7f7f7f, lbound = 0x7f7f7f7f7f7f7f7f, element_type = 0x7f7f7f7f,
typlen = 0x7f7f, typbyval = 0x7f, typalign = 0x7f, dvalues =
0x7f7f7f7f7f7f7f7f, 
  dnulls = 0x7f7f7f7f7f7f7f7f, dvalueslen = 0x7f7f7f7f, nelems = 0x7f7f7f7f,
flat_size = 0x7f7f7f7f7f7f7f7f, fvalue = 0x7f7f7f7f7f7f7f7f, fstartptr =
0x7f7f7f7f7f7f7f7f, fendptr = 0x7f7f7f7f7f7f7f7f}

I discovered this issue with SQLsmith.
Reproduced starting from 1dc5ebc90.



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-24 01:42  Tom Lane <tgl@sss.pgh.pa.us>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2024-11-24 01:42 UTC (permalink / raw)
  To: exclusion@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> writes:
> The following script:
> CREATE FUNCTION make_ia() RETURNS int[] LANGUAGE plpgsql AS
>   'declare x int[]; begin x := array[0]; return x; end';
> CREATE FUNCTION ia_eq(int[], int[]) RETURNS boolean LANGUAGE plpgsql AS
> 'begin return array_eq($1, $2); end'; 
> CREATE OPERATOR = (procedure = ia_eq, leftarg = int[], rightarg = int[]);
> SELECT NULLIF(make_ia(), array[1]::int[]);

> fails with:
> ERROR:  cache lookup failed for type 2139062143

Nice catch!  What is happening here is that make_ia returns a
read/write pointer to an expanded array object.  The EEOP_NULLIF
code passes that pointer straight on to the equality function.
Which in this case is a plpgsql function that will suppose it
can take ownership of the expanded object, resulting in said
object being freed before return.  (Neither function has done
anything wrong.)  The problem is that EEOP_NULLIF then returns
the original Datum pointer, which is now pointing at garbage.
The different failures you get depending on what is done next
with the Datum are not too surprising.

What EEOP_NULLIF needs to do is pass a read-only pointer to the
equality function, so that the object is not modified and remains
available to return if we want to do so.

Attached is a quick WIP patch to handle that.  It is missing a test
case, but the real omission is that llvm_compile_expr()'s EEOP_NULLIF
handling also needs to be fixed, and I'm pretty unsure how to do that.

I'm wondering now if any of our other conditional expressions have
similar bugs ...

			regards, tom lane

Attachments:

bug-18722-fix-wip.patch (text/x-diff, 2.6K) download diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 8f7a534005..69d36f70b3 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1191,6 +1191,14 @@ ExecInitExprRec(Expr *node, ExprState *state, op->args, op->opfuncid, op->inputcollid, state); + /* + * If first argument is of varlena type, we'll need to ensure + * that the value passed to the comparison function is a + * read-only pointer. + */ + scratch.d.func.make_ro = + (get_typlen(exprType((Node *) linitial(op->args))) == -1); + /* * Change opcode of call instruction to EEOP_NULLIF. * diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 30c5a19aad..42da962178 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -1308,12 +1308,24 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) * The arguments are already evaluated into fcinfo->args. */ FunctionCallInfo fcinfo = op->d.func.fcinfo_data; + Datum save_arg0 = fcinfo->args[0].value; /* if either argument is NULL they can't be equal */ if (!fcinfo->args[0].isnull && !fcinfo->args[1].isnull) { Datum result; + /* + * If first argument is of varlena type, it might be an + * expanded datum. We need to ensure that the value passed to + * the comparison function is a read-only pointer. However, + * if we end by returning the first argument, that will be the + * original read-write pointer if it was read-write. + */ + if (op->d.func.make_ro) + fcinfo->args[0].value = + MakeExpandedObjectReadOnlyInternal(save_arg0); + fcinfo->isnull = false; result = op->d.func.fn_addr(fcinfo); @@ -1328,7 +1340,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) } /* Arguments aren't equal, so return the first one */ - *op->resvalue = fcinfo->args[0].value; + *op->resvalue = save_arg0; *op->resnull = fcinfo->args[0].isnull; EEO_NEXT(); diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index cd97dfa062..56fb0d0adb 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -365,6 +365,7 @@ typedef struct ExprEvalStep /* faster to access without additional indirection: */ PGFunction fn_addr; /* actual call address */ int nargs; /* number of arguments */ + bool make_ro; /* make arg0 R/O (used only for NULLIF) */ } func; /* for EEOP_BOOL_*_STEP */
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-24 22:49  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2024-11-24 22:49 UTC (permalink / raw)
  To: exclusion@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org

I wrote:
> What EEOP_NULLIF needs to do is pass a read-only pointer to the
> equality function, so that the object is not modified and remains
> available to return if we want to do so.

> Attached is a quick WIP patch to handle that.  It is missing a test
> case, but the real omission is that llvm_compile_expr()'s EEOP_NULLIF
> handling also needs to be fixed, and I'm pretty unsure how to do that.

Here's a fleshed-out patch with a test case and JIT support.  This
is about the first time I've messed with LLVM, so I wouldn't mind
some review of what I did in llvmjit_expr.c.  In particular, do I
correctly understand that "l_funcvalue(b, v_fcinfo, 0)" produces
a reference to a copy of the initial value of args[0].value?
It seems to work in testing --- the original R/W pointer is returned
out of the expression --- but if we returned the R/O pointer instead
it'd be mighty difficult to detect the resulting inefficiency.

> I'm wondering now if any of our other conditional expressions have
> similar bugs ...

I was amused to discover that case.sql already had infrastructure
suitable for testing this, thanks to bug #14472.

			regards, tom lane

Attachments:

bug-18722-fix-v1.patch (text/x-diff, 5.9K) download diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 8f7a534005..69d36f70b3 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1191,6 +1191,14 @@ ExecInitExprRec(Expr *node, ExprState *state, op->args, op->opfuncid, op->inputcollid, state); + /* + * If first argument is of varlena type, we'll need to ensure + * that the value passed to the comparison function is a + * read-only pointer. + */ + scratch.d.func.make_ro = + (get_typlen(exprType((Node *) linitial(op->args))) == -1); + /* * Change opcode of call instruction to EEOP_NULLIF. * diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 30c5a19aad..42da962178 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -1308,12 +1308,24 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) * The arguments are already evaluated into fcinfo->args. */ FunctionCallInfo fcinfo = op->d.func.fcinfo_data; + Datum save_arg0 = fcinfo->args[0].value; /* if either argument is NULL they can't be equal */ if (!fcinfo->args[0].isnull && !fcinfo->args[1].isnull) { Datum result; + /* + * If first argument is of varlena type, it might be an + * expanded datum. We need to ensure that the value passed to + * the comparison function is a read-only pointer. However, + * if we end by returning the first argument, that will be the + * original read-write pointer if it was read-write. + */ + if (op->d.func.make_ro) + fcinfo->args[0].value = + MakeExpandedObjectReadOnlyInternal(save_arg0); + fcinfo->isnull = false; result = op->d.func.fn_addr(fcinfo); @@ -1328,7 +1340,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) } /* Arguments aren't equal, so return the first one */ - *op->resvalue = fcinfo->args[0].value; + *op->resvalue = save_arg0; *op->resnull = fcinfo->args[0].isnull; EEO_NEXT(); diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 9cec17544b..c533f55254 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1559,6 +1559,9 @@ llvm_compile_expr(ExprState *state) v_fcinfo = l_ptr_const(fcinfo, l_ptr(StructFunctionCallInfoData)); + /* save original arg[0] */ + v_arg0 = l_funcvalue(b, v_fcinfo, 0); + /* if either argument is NULL they can't be equal */ v_argnull0 = l_funcnull(b, v_fcinfo, 0); v_argnull1 = l_funcnull(b, v_fcinfo, 1); @@ -1575,7 +1578,6 @@ llvm_compile_expr(ExprState *state) /* one (or both) of the arguments are null, return arg[0] */ LLVMPositionBuilderAtEnd(b, b_hasnull); - v_arg0 = l_funcvalue(b, v_fcinfo, 0); LLVMBuildStore(b, v_argnull0, v_resnullp); LLVMBuildStore(b, v_arg0, v_resvaluep); LLVMBuildBr(b, opblocks[opno + 1]); @@ -1583,12 +1585,35 @@ llvm_compile_expr(ExprState *state) /* build block to invoke function and check result */ LLVMPositionBuilderAtEnd(b, b_nonull); + /* + * If first argument is of varlena type, it might be an + * expanded datum. We need to ensure that the value + * passed to the comparison function is a read-only + * pointer. However, if we end by returning the first + * argument, that will be the original read-write pointer + * if it was read-write. + */ + if (op->d.func.make_ro) + { + LLVMValueRef v_params[1]; + LLVMValueRef v_arg0_ro; + + v_params[0] = v_arg0; + v_arg0_ro = + l_call(b, + llvm_pg_var_func_type("MakeExpandedObjectReadOnlyInternal"), + llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), + v_params, lengthof(v_params), ""); + LLVMBuildStore(b, v_arg0_ro, + l_funcvaluep(b, v_fcinfo, 0)); + } + v_retval = BuildV1Call(context, b, mod, fcinfo, &v_fcinfo_isnull); /* - * If result not null, and arguments are equal return null - * (same result as if there'd been NULLs, hence reuse - * b_hasnull). + * If result not null and arguments are equal return null, + * else return arg[0] (same result as if there'd been + * NULLs, hence reuse b_hasnull). */ v_argsequal = LLVMBuildAnd(b, LLVMBuildICmp(b, LLVMIntEQ, diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index cd97dfa062..56fb0d0adb 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -365,6 +365,7 @@ typedef struct ExprEvalStep /* faster to access without additional indirection: */ PGFunction fn_addr; /* actual call address */ int nargs; /* number of arguments */ + bool make_ro; /* make arg0 R/O (used only for NULLIF) */ } func; /* for EEOP_BOOL_*_STEP */ diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out index f5136c17ab..efee7fc431 100644 --- a/src/test/regress/expected/case.out +++ b/src/test/regress/expected/case.out @@ -397,6 +397,14 @@ SELECT CASE make_ad(1,2) right (1 row) +-- While we're here, also test handling of a NULLIF arg that is a read/write +-- object (bug #18722) +SELECT NULLIF(make_ad(1,2), array[2,3]::arrdomain); + nullif +-------- + {1,2} +(1 row) + ROLLBACK; -- Test interaction of CASE with ArrayCoerceExpr (bug #15471) BEGIN; diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql index 83fe43be6b..388d4c6f52 100644 --- a/src/test/regress/sql/case.sql +++ b/src/test/regress/sql/case.sql @@ -242,6 +242,11 @@ SELECT CASE make_ad(1,2) WHEN array[1,2]::arrdomain THEN 'right' END; +-- While we're here, also test handling of a NULLIF arg that is a read/write +-- object (bug #18722) + +SELECT NULLIF(make_ad(1,2), array[2,3]::arrdomain); + ROLLBACK; -- Test interaction of CASE with ArrayCoerceExpr (bug #15471)
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-25 10:30  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Dean Rasheed @ 2024-11-25 10:30 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

On Sun, 24 Nov 2024 at 22:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Here's a fleshed-out patch with a test case and JIT support.  This
> is about the first time I've messed with LLVM, so I wouldn't mind
> some review of what I did in llvmjit_expr.c.  In particular, do I
> correctly understand that "l_funcvalue(b, v_fcinfo, 0)" produces
> a reference to a copy of the initial value of args[0].value?
>

I don't know about that, but I wonder if this bug could be fixed by
having ExecInitExprRec() insert a EEOP_MAKE_READONLY step. Then it
wouldn't be necessary to make any changes to the expression evaluation
code.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-25 15:07  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2024-11-25 15:07 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On Sun, 24 Nov 2024 at 22:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Here's a fleshed-out patch with a test case and JIT support.  This
>> is about the first time I've messed with LLVM, so I wouldn't mind
>> some review of what I did in llvmjit_expr.c.  In particular, do I
>> correctly understand that "l_funcvalue(b, v_fcinfo, 0)" produces
>> a reference to a copy of the initial value of args[0].value?

> I don't know about that, but I wonder if this bug could be fixed by
> having ExecInitExprRec() insert a EEOP_MAKE_READONLY step. Then it
> wouldn't be necessary to make any changes to the expression evaluation
> code.

That would entirely destroy one of the primary performance benefits of
the expanded-object infrastructure.  The idea is that if you have
    fconsumer(fproducer(...), ...)
and fproducer returns a read-write pointer to an object it's built,
then fconsumer should be able to take ownership of the object and
use it as a local variable (possibly modifying it) without incurring
any object-copying overhead.

This works in any context where an intermediate expression value
has a single consumer, which is most.  If there are multiple
consumers then we need to insert MAKE_READONLY steps for all
(or all but one) of them.  I overlooked EEOP_NULLIF as such
a case, but I don't think there are so many more cases as to
justify throwing away the concept altogether.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-25 17:59  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Dean Rasheed @ 2024-11-25 17:59 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, 25 Nov 2024 at 15:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> > I don't know about that, but I wonder if this bug could be fixed by
> > having ExecInitExprRec() insert a EEOP_MAKE_READONLY step. Then it
> > wouldn't be necessary to make any changes to the expression evaluation
> > code.
>
> That would entirely destroy one of the primary performance benefits of
> the expanded-object infrastructure.  The idea is that if you have
>     fconsumer(fproducer(...), ...)
> and fproducer returns a read-write pointer to an object it's built,
> then fconsumer should be able to take ownership of the object and
> use it as a local variable (possibly modifying it) without incurring
> any object-copying overhead.
>
> This works in any context where an intermediate expression value
> has a single consumer, which is most.  If there are multiple
> consumers then we need to insert MAKE_READONLY steps for all
> (or all but one) of them.  I overlooked EEOP_NULLIF as such
> a case, but I don't think there are so many more cases as to
> justify throwing away the concept altogether.
>

I didn't mean do it in all cases, I just meant the NullIfExpr case
identified here. My point was that instead of modifying the evaluation
code for EEOP_NULLIF to make it call
MakeExpandedObjectReadOnlyInternal(), it would be easier to insert a
EEOP_MAKE_READONLY step for the first argument of the EEOP_NULLIF
step.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-25 19:16  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2024-11-25 19:16 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> I didn't mean do it in all cases, I just meant the NullIfExpr case
> identified here. My point was that instead of modifying the evaluation
> code for EEOP_NULLIF to make it call
> MakeExpandedObjectReadOnlyInternal(), it would be easier to insert a
> EEOP_MAKE_READONLY step for the first argument of the EEOP_NULLIF
> step.

But then the NULLIF step would only have access to the R/O pointer,
no?  We do want to pass on a R/W pointer to the output, if we got
one, to handle cases like
	fconsumer(NULLIF(fproducer(...), ...), ...)
Admittedly that's a pretty edgy edge-case, but still we're leaving
money on the table if we don't do it.  So I think we have to deal
with the issue within NULLIF.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #18722: Processing arrays with plpgsql raises errors
@ 2024-11-25 20:42  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 8+ messages in thread

From: Dean Rasheed @ 2024-11-25 20:42 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, 25 Nov 2024 at 19:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> > I didn't mean do it in all cases, I just meant the NullIfExpr case
> > identified here. My point was that instead of modifying the evaluation
> > code for EEOP_NULLIF to make it call
> > MakeExpandedObjectReadOnlyInternal(), it would be easier to insert a
> > EEOP_MAKE_READONLY step for the first argument of the EEOP_NULLIF
> > step.
>
> But then the NULLIF step would only have access to the R/O pointer,
> no?  We do want to pass on a R/W pointer to the output, if we got
> one, to handle cases like
>         fconsumer(NULLIF(fproducer(...), ...), ...)
> Admittedly that's a pretty edgy edge-case, but still we're leaving
> money on the table if we don't do it.  So I think we have to deal
> with the issue within NULLIF.
>

OK, that makes sense.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2024-11-25 20:42 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-11-23 10:00 BUG #18722: Processing arrays with plpgsql raises errors PG Bug reporting form <noreply@postgresql.org>
2024-11-24 01:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-24 22:49   ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 10:30     ` Dean Rasheed <dean.a.rasheed@gmail.com>
2024-11-25 15:07       ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 17:59         ` Dean Rasheed <dean.a.rasheed@gmail.com>
2024-11-25 19:16           ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 20:42             ` Dean Rasheed <dean.a.rasheed@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox