Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcd1b215

Browse files
committed
Fix handling of expanded objects in CoerceToDomain and CASE execution.
When the input value to a CoerceToDomain expression node is a read-writeexpanded datum, we should pass a read-only pointer to any domain CHECKexpressions and then return the original read-write pointer as theexpression result. Previously we were blindly passing the same pointer toall the consumers of the value, making it possible for a function in CHECKto modify or even delete the expanded value. (Since a plpgsql functionwill absorb a passed-in read-write expanded array as a local variablevalue, it will in fact delete the value on exit.)A similar hazard of passing the same read-write pointer to multipleconsumers exists in domain_check() and in ExecEvalCase, so fix those too.The fix requires adding MakeExpandedObjectReadOnly calls at the appropriateplaces, which is simple enough except that we need to get the data type'styplen from somewhere. For the domain cases, solve this by redefiningDomainConstraintRef.tcache as okay for callers to access; there wasn't anyreason for the original convention against that, other than not wanting theAPI of typcache.c to be any wider than it had to be. For CASE, there'sno good solution except to add a syscache lookup during executor start.Per bug #14472 from Marcos Castedo. Back-patch to 9.5 where expandedvalues were introduced.Discussion:https://postgr.es/m/15225.1482431619@sss.pgh.pa.us
1 parent6ef2eba commitcd1b215

File tree

8 files changed

+127
-8
lines changed

8 files changed

+127
-8
lines changed

‎src/backend/executor/execQual.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2992,12 +2992,18 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
29922992

29932993
if (caseExpr->arg)
29942994
{
2995+
Datumarg_value;
29952996
boolarg_isNull;
29962997

2997-
econtext->caseValue_datum=ExecEvalExpr(caseExpr->arg,
2998-
econtext,
2999-
&arg_isNull,
3000-
NULL);
2998+
arg_value=ExecEvalExpr(caseExpr->arg,
2999+
econtext,
3000+
&arg_isNull,
3001+
NULL);
3002+
/* Since caseValue_datum may be read multiple times, force to R/O */
3003+
econtext->caseValue_datum=
3004+
MakeExpandedObjectReadOnly(arg_value,
3005+
arg_isNull,
3006+
caseExpr->argtyplen);
30013007
econtext->caseValue_isNull=arg_isNull;
30023008
}
30033009

@@ -4127,11 +4133,18 @@ ExecEvalCoerceToDomain(CoerceToDomainState *cstate, ExprContext *econtext,
41274133
* nodes. We must save and restore prior setting of
41284134
* econtext's domainValue fields, in case this node is
41294135
* itself within a check expression for another domain.
4136+
*
4137+
* Also, if we are working with a read-write expanded
4138+
* datum, be sure that what we pass to CHECK expressions
4139+
* is a read-only pointer; else called functions might
4140+
* modify or even delete the expanded object.
41304141
*/
41314142
save_datum=econtext->domainValue_datum;
41324143
save_isNull=econtext->domainValue_isNull;
41334144

4134-
econtext->domainValue_datum=result;
4145+
econtext->domainValue_datum=
4146+
MakeExpandedObjectReadOnly(result,*isNull,
4147+
cstate->constraint_ref->tcache->typlen);
41354148
econtext->domainValue_isNull=*isNull;
41364149

41374150
conResult=ExecEvalExpr(con->check_expr,
@@ -4939,6 +4952,8 @@ ExecInitExpr(Expr *node, PlanState *parent)
49394952
}
49404953
cstate->args=outlist;
49414954
cstate->defresult=ExecInitExpr(caseexpr->defresult,parent);
4955+
if (caseexpr->arg)
4956+
cstate->argtyplen=get_typlen(exprType((Node*)caseexpr->arg));
49424957
state= (ExprState*)cstate;
49434958
}
49444959
break;

‎src/backend/utils/adt/domains.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include"executor/executor.h"
3636
#include"lib/stringinfo.h"
3737
#include"utils/builtins.h"
38+
#include"utils/expandeddatum.h"
3839
#include"utils/lsyscache.h"
3940
#include"utils/syscache.h"
4041
#include"utils/typcache.h"
@@ -166,9 +167,14 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra)
166167
* Set up value to be returned by CoerceToDomainValue
167168
* nodes. Unlike ExecEvalCoerceToDomain, this econtext
168169
* couldn't be shared with anything else, so no need to
169-
* save and restore fields.
170+
* save and restore fields. But we do need to protect the
171+
* passed-in value against being changed by called
172+
* functions. (It couldn't be a R/W expanded object for
173+
* most uses, but that seems possible for domain_check().)
170174
*/
171-
econtext->domainValue_datum=value;
175+
econtext->domainValue_datum=
176+
MakeExpandedObjectReadOnly(value,isnull,
177+
my_extra->constraint_ref.tcache->typlen);
172178
econtext->domainValue_isNull=isnull;
173179

174180
conResult=ExecEvalExprSwitchContext(con->check_expr,

‎src/include/nodes/execnodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ typedef struct CaseExprState
889889
ExprState*arg;/* implicit equality comparison argument */
890890
List*args;/* the arguments (list of WHEN clauses) */
891891
ExprState*defresult;/* the default result (ELSE clause) */
892+
int16argtyplen;/* if arg is provided, its typlen */
892893
}CaseExprState;
893894

894895
/* ----------------

‎src/include/utils/typcache.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,9 @@ typedef struct DomainConstraintRef
131131
{
132132
List*constraints;/* list of DomainConstraintState nodes */
133133
MemoryContextrefctx;/* context holding DomainConstraintRef */
134+
TypeCacheEntry*tcache;/* typcache entry for domain type */
134135

135136
/* Management data --- treat these fields as private to typcache.c */
136-
TypeCacheEntry*tcache;/* owning typcache entry */
137137
DomainConstraintCache*dcc;/* current constraints, or NULL if none */
138138
MemoryContextCallbackcallback;/* used to release refcount when done */
139139
}DomainConstraintRef;

‎src/test/regress/expected/case.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,32 @@ SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo'
338338
is not foo
339339
(1 row)
340340

341+
ROLLBACK;
342+
-- Test multiple evaluation of a CASE arg that is a read/write object (#14472)
343+
-- Wrap this in a single transaction so the transient '=' operator doesn't
344+
-- cause problems in concurrent sessions
345+
BEGIN;
346+
CREATE DOMAIN arrdomain AS int[];
347+
CREATE FUNCTION make_ad(int,int) returns arrdomain as
348+
'declare x arrdomain;
349+
begin
350+
x := array[$1,$2];
351+
return x;
352+
end' language plpgsql volatile;
353+
CREATE FUNCTION ad_eq(arrdomain, arrdomain) returns boolean as
354+
'begin return array_eq($1, $2); end' language plpgsql;
355+
CREATE OPERATOR = (procedure = ad_eq,
356+
leftarg = arrdomain, rightarg = arrdomain);
357+
SELECT CASE make_ad(1,2)
358+
WHEN array[2,4]::arrdomain THEN 'wrong'
359+
WHEN array[2,5]::arrdomain THEN 'still wrong'
360+
WHEN array[1,2]::arrdomain THEN 'right'
361+
END;
362+
case
363+
-------
364+
right
365+
(1 row)
366+
341367
ROLLBACK;
342368
--
343369
-- Clean up

‎src/test/regress/expected/plpgsql.out

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5664,3 +5664,23 @@ end;
56645664
$$;
56655665
ERROR: value for domain plpgsql_domain violates check constraint "plpgsql_domain_check"
56665666
CONTEXT: PL/pgSQL function inline_code_block line 4 at assignment
5667+
-- Test handling of expanded array passed to a domain constraint (bug #14472)
5668+
create function plpgsql_arr_domain_check(val int[]) returns boolean as $$
5669+
begin return val[1] > 0; end
5670+
$$ language plpgsql immutable;
5671+
create domain plpgsql_arr_domain as int[] check(plpgsql_arr_domain_check(value));
5672+
do $$
5673+
declare v_test plpgsql_arr_domain;
5674+
begin
5675+
v_test := array[1];
5676+
v_test := v_test || 2;
5677+
end;
5678+
$$;
5679+
do $$
5680+
declare v_test plpgsql_arr_domain := array[1];
5681+
begin
5682+
v_test := 0 || v_test; -- fail
5683+
end;
5684+
$$;
5685+
ERROR: value for domain plpgsql_arr_domain violates check constraint "plpgsql_arr_domain_check"
5686+
CONTEXT: PL/pgSQL function inline_code_block line 4 at assignment

‎src/test/regress/sql/case.sql

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,34 @@ SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo'
200200

201201
ROLLBACK;
202202

203+
-- Test multiple evaluation of a CASE arg that is a read/write object (#14472)
204+
-- Wrap this in a single transaction so the transient '=' operator doesn't
205+
-- cause problems in concurrent sessions
206+
BEGIN;
207+
208+
CREATEDOMAINarrdomainASint[];
209+
210+
CREATEFUNCTIONmake_ad(int,int) returns arrdomainas
211+
'declare x arrdomain;
212+
begin
213+
x := array[$1,$2];
214+
return x;
215+
end' language plpgsql volatile;
216+
217+
CREATEFUNCTIONad_eq(arrdomain, arrdomain) returnsbooleanas
218+
'begin return array_eq($1, $2); end' language plpgsql;
219+
220+
CREATE OPERATOR= (procedure= ad_eq,
221+
leftarg= arrdomain, rightarg= arrdomain);
222+
223+
SELECT CASE make_ad(1,2)
224+
WHEN array[2,4]::arrdomain THEN'wrong'
225+
WHEN array[2,5]::arrdomain THEN'still wrong'
226+
WHEN array[1,2]::arrdomain THEN'right'
227+
END;
228+
229+
ROLLBACK;
230+
203231
--
204232
-- Clean up
205233
--

‎src/test/regress/sql/plpgsql.sql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4452,3 +4452,26 @@ begin
44524452
v_test :=0;-- fail
44534453
end;
44544454
$$;
4455+
4456+
-- Test handling of expanded array passed to a domain constraint (bug #14472)
4457+
4458+
createfunctionplpgsql_arr_domain_check(valint[]) returnsbooleanas $$
4459+
begin return val[1]>0; end
4460+
$$ language plpgsql immutable;
4461+
4462+
createdomainplpgsql_arr_domainasint[]check(plpgsql_arr_domain_check(value));
4463+
4464+
do $$
4465+
declare v_test plpgsql_arr_domain;
4466+
begin
4467+
v_test := array[1];
4468+
v_test := v_test||2;
4469+
end;
4470+
$$;
4471+
4472+
do $$
4473+
declare v_test plpgsql_arr_domain := array[1];
4474+
begin
4475+
v_test :=0|| v_test;-- fail
4476+
end;
4477+
$$;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp