plpgsql_check_function - implementation
Started by Pavel Stehulealmost 13 years ago1 messages
Hello
I try to play with different implementations of plpgsql deep checking.
The most important task of deep checking is creating plans for all
queries and expressions in function. The prerequisite for this task is
knowledge of data types of all variables. Record and row types is
break, but there is workaround - we are able to derive data types from
plans and we can assign with high success rate valid types to this
kind variables. We are not able to do with result of dynamic SQL and
temporary tables still - just we are not able to detect possible
errors for dynamic queries ever.
There are four possible implementations:
0) special recursive check routine + derivation data types from plans:
+ zero impact on current code, readability, - one other long recursive
routine
a) enhance parser + derivation data types from plans: + no new
recursive routine, - order of check depends on bison processing order,
result needs a final sort
b) enhance executor nodes + take data types from fake execution: +
relative less new code, - decrease readability of executor code, 20%
slowdown of CPU bottle neck code (new code is on critical path)
I tested code (this is a worst situation) - patch is in attachment (it
is WIP - just for test of impact new code to performance)
CREATE OR REPLACE FUNCTION public.test()
RETURNS integer
LANGUAGE plpgsql
IMMUTABLE
AS $function$
declare i int;
declare j int;
begin
i := 1;
while i < 10000 loop
j := 1;
while j < 1000 loop
j := j + 1;
end loop;
i := i + 1;
end loop;
return i;
end;
$function$
c) merge checking and dumping and derivation data from plans: + zero
impact on current code, readability, - some new code
my @0 works well, but was repeatedly rejected by Tom and Heikki, @a
needs final sort - so it needs more complex infrastructure for
creating result tuplestore, @b has mensurable performance impact
(from 9454 to 11274 ms), so there are only @c.
comments, notices?
Regards
Pavel
Attachments:
plpgsql_check_implementation.patchapplication/octet-stream; name=plpgsql_check_implementation.patchDownload
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 1321,1326 **** static int
--- 1321,1327 ----
exec_stmts(PLpgSQL_execstate *estate, List *stmts)
{
ListCell *s;
+ bool touch_all = estate->checker != NULL;
if (stmts == NIL)
{
***************
*** 1338,1344 **** exec_stmts(PLpgSQL_execstate *estate, List *stmts)
PLpgSQL_stmt *stmt = (PLpgSQL_stmt *) lfirst(s);
int rc = exec_stmt(estate, stmt);
! if (rc != PLPGSQL_RC_OK)
return rc;
}
--- 1339,1345 ----
PLpgSQL_stmt *stmt = (PLpgSQL_stmt *) lfirst(s);
int rc = exec_stmt(estate, stmt);
! if (rc != PLPGSQL_RC_OK && !touch_all)
return rc;
}
***************
*** 1592,1605 **** exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
static int
exec_stmt_if(PLpgSQL_execstate *estate, PLpgSQL_stmt_if *stmt)
{
bool value;
bool isnull;
ListCell *lc;
value = exec_eval_boolean(estate, stmt->cond, &isnull);
exec_eval_cleanup(estate);
! if (!isnull && value)
! return exec_stmts(estate, stmt->then_body);
foreach(lc, stmt->elsif_list)
{
--- 1593,1612 ----
static int
exec_stmt_if(PLpgSQL_execstate *estate, PLpgSQL_stmt_if *stmt)
{
+ int rc;
bool value;
bool isnull;
+ bool touch_all = estate->checker != NULL;
ListCell *lc;
value = exec_eval_boolean(estate, stmt->cond, &isnull);
exec_eval_cleanup(estate);
! if ((!isnull && value) || touch_all)
! {
! rc = exec_stmts(estate, stmt->then_body);
! if (!touch_all)
! return rc;
! }
foreach(lc, stmt->elsif_list)
{
***************
*** 1607,1614 **** exec_stmt_if(PLpgSQL_execstate *estate, PLpgSQL_stmt_if *stmt)
value = exec_eval_boolean(estate, elif->cond, &isnull);
exec_eval_cleanup(estate);
! if (!isnull && value)
! return exec_stmts(estate, elif->stmts);
}
return exec_stmts(estate, stmt->else_body);
--- 1614,1625 ----
value = exec_eval_boolean(estate, elif->cond, &isnull);
exec_eval_cleanup(estate);
! if ((!isnull && value) || touch_all)
! {
! rc = exec_stmts(estate, elif->stmts);
! if (!touch_all)
! return rc;
! }
}
return exec_stmts(estate, stmt->else_body);
***************
*** 1622,1629 **** exec_stmt_if(PLpgSQL_execstate *estate, PLpgSQL_stmt_if *stmt)
--- 1633,1642 ----
static int
exec_stmt_case(PLpgSQL_execstate *estate, PLpgSQL_stmt_case *stmt)
{
+ int rc;
PLpgSQL_var *t_var = NULL;
bool isnull;
+ bool touch_all = estate->checker != NULL;
ListCell *l;
if (stmt->t_expr != NULL)
***************
*** 1664,1670 **** exec_stmt_case(PLpgSQL_execstate *estate, PLpgSQL_stmt_case *stmt)
value = exec_eval_boolean(estate, cwt->expr, &isnull);
exec_eval_cleanup(estate);
! if (!isnull && value)
{
/* Found it */
--- 1677,1683 ----
value = exec_eval_boolean(estate, cwt->expr, &isnull);
exec_eval_cleanup(estate);
! if ((!isnull && value) || touch_all)
{
/* Found it */
***************
*** 1677,1683 **** exec_stmt_case(PLpgSQL_execstate *estate, PLpgSQL_stmt_case *stmt)
}
/* Evaluate the statement(s), and we're done */
! return exec_stmts(estate, cwt->stmts);
}
}
--- 1690,1698 ----
}
/* Evaluate the statement(s), and we're done */
! rc = exec_stmts(estate, cwt->stmts);
! if (!touch_all)
! return rc;
}
}
***************
*** 1690,1696 **** exec_stmt_case(PLpgSQL_execstate *estate, PLpgSQL_stmt_case *stmt)
}
/* SQL2003 mandates this error if there was no ELSE clause */
! if (!stmt->have_else)
ereport(ERROR,
(errcode(ERRCODE_CASE_NOT_FOUND),
errmsg("case not found"),
--- 1705,1711 ----
}
/* SQL2003 mandates this error if there was no ELSE clause */
! if (!stmt->have_else && !touch_all)
ereport(ERROR,
(errcode(ERRCODE_CASE_NOT_FOUND),
errmsg("case not found"),
***************
*** 1709,1714 **** exec_stmt_case(PLpgSQL_execstate *estate, PLpgSQL_stmt_case *stmt)
--- 1724,1731 ----
static int
exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
{
+ bool touch_all = estate->checker != NULL;
+
for (;;)
{
int rc = exec_stmts(estate, stmt->body);
***************
*** 1747,1752 **** exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
--- 1764,1773 ----
default:
elog(ERROR, "unrecognized rc: %d", rc);
}
+
+ /* only one loop in touch_all mode */
+ if (touch_all)
+ return rc;
}
}
***************
*** 1760,1765 **** exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
--- 1781,1788 ----
static int
exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
{
+ bool touch_all = estate->checker != NULL;
+
for (;;)
{
int rc;
***************
*** 1769,1775 **** exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
value = exec_eval_boolean(estate, stmt->cond, &isnull);
exec_eval_cleanup(estate);
! if (isnull || !value)
break;
rc = exec_stmts(estate, stmt->body);
--- 1792,1798 ----
value = exec_eval_boolean(estate, stmt->cond, &isnull);
exec_eval_cleanup(estate);
! if ((isnull || !value) && !touch_all)
break;
rc = exec_stmts(estate, stmt->body);
***************
*** 1808,1813 **** exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
--- 1831,1840 ----
default:
elog(ERROR, "unrecognized rc: %d", rc);
}
+
+ /* only one loop in touch all mode */
+ if (touch_all)
+ return PLPGSQL_RC_EXIT;
}
return PLPGSQL_RC_OK;
***************
*** 1832,1837 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
--- 1859,1865 ----
int32 step_value;
bool found = false;
int rc = PLPGSQL_RC_OK;
+ bool touch_all = estate->checker != NULL;
var = (PLpgSQL_var *) (estate->datums[stmt->var->dno]);
***************
*** 1843,1854 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("lower bound of FOR loop cannot be null")));
! loop_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
/*
* Get the value of the upper bound
--- 1871,1886 ----
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
!
! if (!touch_all)
! {
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("lower bound of FOR loop cannot be null")));
! loop_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
! }
/*
* Get the value of the upper bound
***************
*** 1858,1869 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("upper bound of FOR loop cannot be null")));
! end_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
/*
* Get the step value
--- 1890,1904 ----
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
! if (!touch_all)
! {
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("upper bound of FOR loop cannot be null")));
! end_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
! }
/*
* Get the step value
***************
*** 1875,1890 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("BY value of FOR loop cannot be null")));
! step_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
! if (step_value <= 0)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("BY value of FOR loop must be greater than zero")));
}
else
step_value = 1;
--- 1910,1929 ----
&(var->datatype->typinput),
var->datatype->typioparam,
var->datatype->atttypmod, isnull);
!
! if (!touch_all)
! {
! if (isnull)
! ereport(ERROR,
! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
! errmsg("BY value of FOR loop cannot be null")));
! step_value = DatumGetInt32(value);
! exec_eval_cleanup(estate);
! if (step_value <= 0)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("BY value of FOR loop must be greater than zero")));
! }
}
else
step_value = 1;
***************
*** 1897,1911 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
/*
* Check against upper bound
*/
! if (stmt->reverse)
{
! if (loop_value < end_value)
! break;
! }
! else
! {
! if (loop_value > end_value)
! break;
}
found = true; /* looped at least once */
--- 1936,1953 ----
/*
* Check against upper bound
*/
! if (!touch_all)
{
! if (stmt->reverse)
! {
! if (loop_value < end_value)
! break;
! }
! else
! {
! if (loop_value > end_value)
! break;
! }
}
found = true; /* looped at least once */
***************
*** 1913,1920 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
/*
* Assign current value to loop var
*/
! var->value = Int32GetDatum(loop_value);
! var->isnull = false;
/*
* Execute the statements
--- 1955,1965 ----
/*
* Assign current value to loop var
*/
! if (!touch_all)
! {
! var->value = Int32GetDatum(loop_value);
! var->isnull = false;
! }
/*
* Execute the statements
***************
*** 1966,1971 **** exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
--- 2011,2020 ----
}
}
+ /* only one loop in touch_all mode */
+ if (touch_all)
+ return PLPGSQL_RC_EXIT;
+
/*
* Increase/decrease loop value, unless it would overflow, in which
* case exit the loop.
***************
*** 3013,3018 **** plpgsql_estate_setup(PLpgSQL_execstate *estate,
--- 3062,3069 ----
estate->func = func;
+ estate->checker = NULL;
+
estate->retval = (Datum) 0;
estate->retisnull = true;
estate->rettype = InvalidOid;
***************
*** 3850,3855 **** exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
--- 3901,3912 ----
value = exec_eval_expr(estate, expr, &isnull, &valtype);
exec_assign_value(estate, target, value, valtype, &isnull);
exec_eval_cleanup(estate);
+
+ /* simulate checker call */
+ if (estate->checker != NULL)
+ {
+ fprintf(stderr, "assign statement call\n");
+ }
}
***************
*** 5069,5080 **** exec_eval_simple_expr(PLpgSQL_execstate *estate,
expr->expr_simple_in_use = true;
/*
! * Finally we can call the executor to evaluate the expression
*/
! *result = ExecEvalExpr(expr->expr_simple_state,
! econtext,
! isNull,
! NULL);
/* Assorted cleanup */
expr->expr_simple_in_use = false;
--- 5126,5146 ----
expr->expr_simple_in_use = true;
/*
! * Finally we can call the executor to evaluate the expression.
! * Don't would to call the executor in touch_all mode.
*/
! if (estate->checker == NULL)
! {
! *result = ExecEvalExpr(expr->expr_simple_state,
! econtext,
! isNull,
! NULL);
! }
! else
! {
! *result = (Datum) 0;
! *isNull = true;
! }
/* Assorted cleanup */
expr->expr_simple_in_use = false;
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 735,744 **** typedef struct PLpgSQL_function
--- 735,750 ----
unsigned long use_count;
} PLpgSQL_function;
+ typedef struct PLpgSQL_checker
+ {
+ bool perf_warning;
+ } PLpgSQL_checker;
+
typedef struct PLpgSQL_execstate
{ /* Runtime execution data */
PLpgSQL_function *func; /* function being executed */
+ PLpgSQL_checker *checker;
Datum retval;
bool retisnull;