Attached is a patch that makes the following changes:
(1) refactor execQual.c to expose a function for checking an instance of
a domain value against a list of domain constraints
(2) adjust pl/pgsql to fetch the constraints associated with the
function's return value. Because this is expensive, the constraints are
fetched once per session, when the function is compiled. I then modified
pl/pgsql to check any applicable constraints on the return value of a
function before returning it to the backend.
(3) add some regression tests for #2
The patch does NOT add checking of domain constraints for other PLs, or
for intermediate values in PL/PgSQL -- I plan to take a look at doing
one or both of those soon.
I also made a few semi-related cleanups. In pl_exec.c, it seems to me
that estate.eval_econtext MUST be non-NULL during the guts of both
plpgsql_exec_trigger() and plpgsql_exec_function(). Therefore checking
that estate.eval_econtext is non-NULL when cleaning up is unnecessary
(line 649 and 417 in current sources, respectively), so I've removed the
checks. Am I missing something?
Barring any objections I'll apply this patch tomorrow some time.
-Neil
============================================================
*** src/backend/executor/execQual.c eb1daef85341439578a3f6bb73549c4420292bc3
--- src/backend/executor/execQual.c 20fa43d2335a50a1663c8e3e1d7d9e5d091dc79f
***************
*** 2673,2679 ****
{
CoerceToDomain *ctest = (CoerceToDomain *) cstate->xprstate.expr;
Datum result;
- ListCell *l;
result = ExecEvalExpr(cstate->arg, econtext, isNull, isDone);
--- 2673,2678 ----
***************
*** 2680,2686 ****
if (isDone && *isDone == ExprEndResult)
return result; /* nothing to check */
! foreach(l, cstate->constraints)
{
DomainConstraintState *con = (DomainConstraintState *) lfirst(l);
--- 2679,2707 ----
if (isDone && *isDone == ExprEndResult)
return result; /* nothing to check */
! ExecCheckDomainConstraints(result, *isNull, cstate->constraints,
! ctest->resulttype, econtext);
!
! /* No constraints failed, so return the datum */
! return result;
! }
!
! /*
! * Check a list of domain constraints against an instance of the
! * domain type, 'value'. 'isnull' indicates whether the instance is
! * null. 'constraints' is a list of DomainConstraintState nodes
! * describing the constraints to check. 'valtype' is the OID of the
! * domain type. 'econtext' is the ExprContext in which the constraint
! * expressions are evaluated.
! */
! void
! ExecCheckDomainConstraints(Datum value, bool isnull,
! List *constraints, Oid valtype,
! ExprContext *econtext)
! {
! ListCell *l;
!
! foreach(l, constraints)
{
DomainConstraintState *con = (DomainConstraintState *) lfirst(l);
***************
*** 2687,2697 ****
switch (con->constrainttype)
{
case DOM_CONSTRAINT_NOTNULL:
! if (*isNull)
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("domain %s does not allow null values",
! format_type_be(ctest->resulttype))));
break;
case DOM_CONSTRAINT_CHECK:
{
--- 2708,2718 ----
switch (con->constrainttype)
{
case DOM_CONSTRAINT_NOTNULL:
! if (isnull)
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("domain %s does not allow null values",
! format_type_be(valtype))));
break;
case DOM_CONSTRAINT_CHECK:
{
***************
*** 2709,2726 ****
save_datum = econtext->domainValue_datum;
save_isNull = econtext->domainValue_isNull;
! econtext->domainValue_datum = result;
! econtext->domainValue_isNull = *isNull;
conResult = ExecEvalExpr(con->check_expr,
econtext, &conIsNull, NULL);
! if (!conIsNull &&
! !DatumGetBool(conResult))
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("value for domain %s violates check constraint \"%s\"",
! format_type_be(ctest->resulttype),
con->name)));
econtext->domainValue_datum = save_datum;
econtext->domainValue_isNull = save_isNull;
--- 2730,2746 ----
save_datum = econtext->domainValue_datum;
save_isNull = econtext->domainValue_isNull;
! econtext->domainValue_datum = value;
! econtext->domainValue_isNull = isnull;
conResult = ExecEvalExpr(con->check_expr,
econtext, &conIsNull, NULL);
! if (!conIsNull && !DatumGetBool(conResult))
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("value for domain %s violates check constraint \"%s\"",
! format_type_be(valtype),
con->name)));
econtext->domainValue_datum = save_datum;
econtext->domainValue_isNull = save_isNull;
***************
*** 2733,2741 ****
break;
}
}
-
- /* If all has gone well (constraints did not fail) return the datum */
- return result;
}
/*
--- 2753,2758 ----
============================================================
*** src/include/executor/executor.h 502cb98a7719bfb0236c04c8ea7aa999026dc584
--- src/include/executor/executor.h b111913a7ef4735321f09c7686dd95a03b86910f
***************
*** 136,141 ****
--- 136,144 ----
extern int ExecCleanTargetListLength(List *targetlist);
extern TupleTableSlot *ExecProject(ProjectionInfo *projInfo,
ExprDoneCond *isDone);
+ extern void ExecCheckDomainConstraints(Datum value, bool isnull,
+ List *constraints, Oid valtype,
+ ExprContext *econtext);
/*
* prototypes from functions in execScan.c
============================================================
*** src/pl/plpgsql/src/pl_comp.c 425488af1f2bb20c51747073b9837369c846fe38
--- src/pl/plpgsql/src/pl_comp.c baf5c26e94285de9529694a1d891e8c7c8119748
***************
*** 48,53 ****
--- 48,54 ----
#include "catalog/pg_class.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+ #include "commands/typecmds.h"
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "parser/gramparse.h"
***************
*** 344,350 ****
switch (functype)
{
case T_FUNCTION:
-
/*
* Fetch info about the procedure's parameters. Allocations aren't
* needed permanently, so make them in tmp cxt.
--- 345,350 ----
***************
*** 534,539 ****
--- 534,547 ----
true);
}
}
+
+ /*
+ * If the function returns a domain value, lookup and
+ * store any constraints associated with the domain.
+ */
+ if (typeStruct->typtype == 'd')
+ function->domain_constr = GetDomainConstraints(rettypeid);
+
ReleaseSysCache(typeTup);
break;
============================================================
*** src/pl/plpgsql/src/pl_exec.c 36a133fa66455f08b44cda267840ad15017b2c6c
--- src/pl/plpgsql/src/pl_exec.c fa72a8b7cea04dc747541f4a4fb4c6fde6c3b2e3
***************
*** 413,421 ****
}
}
/* Clean up any leftover temporary memory */
! if (estate.eval_econtext != NULL)
! FreeExprContext(estate.eval_econtext);
estate.eval_econtext = NULL;
exec_eval_cleanup(&estate);
--- 413,426 ----
}
}
+ /* Check any applicable domain constraints */
+ if (func->domain_constr != NIL)
+ ExecCheckDomainConstraints(estate.retval, estate.retisnull,
+ func->domain_constr, func->fn_rettype,
+ estate.eval_econtext);
+
/* Clean up any leftover temporary memory */
! FreeExprContext(estate.eval_econtext);
estate.eval_econtext = NULL;
exec_eval_cleanup(&estate);
***************
*** 646,653 ****
}
/* Clean up any leftover temporary memory */
! if (estate.eval_econtext != NULL)
! FreeExprContext(estate.eval_econtext);
estate.eval_econtext = NULL;
exec_eval_cleanup(&estate);
--- 651,657 ----
}
/* Clean up any leftover temporary memory */
! FreeExprContext(estate.eval_econtext);
estate.eval_econtext = NULL;
exec_eval_cleanup(&estate);
***************
*** 3195,3201 ****
errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
/*
! * Get the number of the records field to change and the
* number of attributes in the tuple.
*/
fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
--- 3199,3205 ----
errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
/*
! * Get the number of the record's field to change and the
* number of attributes in the tuple.
*/
fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
***************
*** 4100,4107 ****
if (!isnull)
{
/*
! * If the type of the queries return value isn't that of the variable,
! * convert it.
*/
if (valtype != reqtype || reqtypmod != -1)
{
--- 4104,4111 ----
if (!isnull)
{
/*
! * If the type of the query's return value isn't that of the
! * variable, convert it.
*/
if (valtype != reqtype || reqtypmod != -1)
{
============================================================
*** src/pl/plpgsql/src/plpgsql.h 40743f1288e49eb6a6aedf3ddb0846b598d7319b
--- src/pl/plpgsql/src/plpgsql.h 064138e4024ab55c179f1999327b0dd7324bdc79
***************
*** 568,573 ****
--- 568,574 ----
int fn_functype;
PLpgSQL_func_hashkey *fn_hashkey; /* back-link to hashtable key */
MemoryContext fn_cxt;
+ List *domain_constr; /* Domain constraints on the return value */
Oid fn_rettype;
int fn_rettyplen;
============================================================
*** src/test/regress/expected/plpgsql.out 3e546643943140cd0a07045bccf1c59f51d63fa2
--- src/test/regress/expected/plpgsql.out 9897920b146f632ba4b77ee425f75709bd1f1710
***************
*** 2721,2723 ****
--- 2721,2750 ----
$$ language plpgsql;
ERROR: end label "outer_label" specified for unlabelled block
CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
+ -- check that domain constraints on the function's return value are
+ -- enforced
+ CREATE DOMAIN foo_domain AS INT4 NOT NULL CHECK (VALUE < 10);
+ CREATE FUNCTION domain_check() RETURNS foo_domain AS $$
+ DECLARE
+ x int;
+ y int;
+ BEGIN
+ x := 35;
+ y := 15;
+ RETURN x - y;
+ END;
+ $$ LANGUAGE plpgsql;
+ SELECT domain_check(); -- should fail
+ ERROR: value for domain foo_domain violates check constraint "foo_domain_check"
+ CONTEXT: PL/pgSQL function "domain_check" while casting return value to function's return type
+ CREATE FUNCTION domain_check_null() RETURNS foo_domain AS $$
+ BEGIN
+ RETURN NULL;
+ END;
+ $$ LANGUAGE plpgsql;
+ SELECT domain_check_null(); -- should fail
+ ERROR: domain foo_domain does not allow null values
+ CONTEXT: PL/pgSQL function "domain_check_null" while casting return value to function's return type
+ DROP DOMAIN foo_domain CASCADE;
+ NOTICE: drop cascades to function domain_check_null()
+ NOTICE: drop cascades to function domain_check()
============================================================
*** src/test/regress/sql/plpgsql.sql e8f2eb786ca15e60b817fc839cc412500ee5a693
--- src/test/regress/sql/plpgsql.sql 3d3ce2e48d253638b307d4bc14f549348491a299
***************
*** 2280,2282 ****
--- 2280,2309 ----
end loop outer_label;
end;
$$ language plpgsql;
+
+ -- check that domain constraints on the function's return value are
+ -- enforced
+ CREATE DOMAIN foo_domain AS INT4 NOT NULL CHECK (VALUE < 10);
+
+ CREATE FUNCTION domain_check() RETURNS foo_domain AS $$
+ DECLARE
+ x int;
+ y int;
+ BEGIN
+ x := 35;
+ y := 15;
+ RETURN x - y;
+ END;
+ $$ LANGUAGE plpgsql;
+
+ SELECT domain_check(); -- should fail
+
+ CREATE FUNCTION domain_check_null() RETURNS foo_domain AS $$
+ BEGIN
+ RETURN NULL;
+ END;
+ $$ LANGUAGE plpgsql;
+
+ SELECT domain_check_null(); -- should fail
+
+ DROP DOMAIN foo_domain CASCADE;
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
