Permalink
Browse files

possibility to identify hidden casts in WHERE clauses. It typical sym…

…ptom of variable

used for filtering of some attribute and with different type than attribute.
  • Loading branch information...
okbob committed Jun 5, 2018
1 parent cf41e48 commit 68b899e30f4dd2658d01135b264bba6e7202235b
Showing with 278 additions and 0 deletions.
  1. +66 −0 expected/plpgsql_check_active_1.out
  2. +169 −0 plpgsql_check.c
  3. +43 −0 sql/plpgsql_check_active.sql
@@ -2685,3 +2685,69 @@ select * from plpgsql_check_function('test()');
------------------------
(0 rows)
drop function test();
-- issue #32
create table bigtable(id bigint, v varchar);
create or replace function test()
returns void as $$
declare
r record;
_id numeric;
begin
select * into r from bigtable where id = _id;
for r in select * from bigtable where _id = id
loop
end loop;
if (exists(select * from bigtable where id = _id)) then
end if;
end;
$$ language plpgsql;
select test();
test
------
(1 row)
-- should to show performance warnings
select * from plpgsql_check_function('test()', performance_warnings => true);
plpgsql_check_function
-------------------------------------------------------------------------------------------------------------------------------
performance:42804:6:SQL statement:implicit cast of attribute caused by different PLpgSQL variable type in WHERE clause
Query: select * from bigtable where id = _id
-- ^
Detail: An index of some attribute cannot be used, when variable, used in predicate, has not right type like a attribute
Hint: Check a variable type - int versus numeric
performance:42804:7:FOR over SELECT rows:implicit cast of attribute caused by different PLpgSQL variable type in WHERE clause
Query: select * from bigtable where _id = id
-- ^
Detail: An index of some attribute cannot be used, when variable, used in predicate, has not right type like a attribute
Hint: Check a variable type - int versus numeric
performance:42804:10:IF:implicit cast of attribute caused by different PLpgSQL variable type in WHERE clause
Query: SELECT (exists(select * from bigtable where id = _id))
-- ^
Detail: An index of some attribute cannot be used, when variable, used in predicate, has not right type like a attribute
Hint: Check a variable type - int versus numeric
warning:00000:3:DECLARE:never read variable "r"
(16 rows)
create or replace function test()
returns void as $$
declare
r record;
_id bigint;
begin
select * into r from bigtable where id = _id;
for r in select * from bigtable where _id = id
loop
end loop;
if (exists(select * from bigtable where id = _id)) then
end if;
end;
$$ language plpgsql;
-- there are not any performance issue now
select * from plpgsql_check_function('test()', performance_warnings => true);
plpgsql_check_function
-------------------------------------------------
warning:00000:3:DECLARE:never read variable "r"
(1 row)
View
@@ -252,6 +252,7 @@ static int load_configuration(HeapTuple procTuple, bool *reload_config);
static void mark_as_checked(PLpgSQL_function *func);
static void plpgsql_check_HashTableInit(void);
static void prohibit_write_plan(PLpgSQL_checkstate *cstate, PLpgSQL_expr *query);
static void check_fishy_qual(PLpgSQL_checkstate *cstate, PLpgSQL_expr *query);
static void put_error(PLpgSQL_checkstate *cstate,
int sqlerrcode, int lineno,
const char *message, const char *detail, const char *hint,
@@ -4463,6 +4464,9 @@ prepare_expr(PLpgSQL_checkstate *cstate,
}
}
/*
* We would to check all plans, but when plan exists, then don't
* overwrite existing plan.
@@ -4480,6 +4484,9 @@ prepare_expr(PLpgSQL_checkstate *cstate,
if (cstate->estate->readonly_func)
prohibit_write_plan(cstate, expr);
if (cstate->performance_warnings)
check_fishy_qual(cstate, expr);
prohibit_transaction_stmt(cstate, expr);
}
@@ -4723,6 +4730,97 @@ assign_tupdesc_row_or_rec(PLpgSQL_checkstate *cstate,
}
}
static bool
contain_fishy_cast_walker(Node *node, void *context)
{
if (node == NULL)
return false;
if (IsA(node, OpExpr))
{
OpExpr *opexpr = (OpExpr *) node;
if (!opexpr->opretset && opexpr->opresulttype == BOOLOID
&& list_length(opexpr->args) == 2)
{
Node *l1 = linitial(opexpr->args);
Node *l2 = lsecond(opexpr->args);
Param *param = NULL;
FuncExpr *fexpr = NULL;
if (IsA(l1, Param))
param = (Param *) l1;
else if (IsA(l1, FuncExpr))
fexpr = (FuncExpr *) l1;
if (IsA(l2, Param))
param = (Param *) l2;
else if (IsA(l2, FuncExpr))
fexpr = (FuncExpr *) l2;
if (param != NULL && fexpr != NULL)
{
if (param->paramkind != PARAM_EXTERN)
return false;
if (fexpr->funcformat != COERCE_IMPLICIT_CAST ||
fexpr->funcretset ||
list_length(fexpr->args) != 1 ||
param->paramtype != fexpr->funcresulttype)
return false;
if (!IsA(linitial(fexpr->args), Var))
return false;
*((Param **) context) = param;
return true;
}
}
}
return expression_tree_walker(node, contain_fishy_cast_walker, context);
}
/*
* Try to identify constraint where variable from one side is implicitly casted to
* parameter type of second side. It can symptom of parameter wrong type.
*
*/
static bool
contain_fishy_cast(Node *node, Param **param)
{
return contain_fishy_cast_walker(node, param);
}
static bool
qual_has_fishy_cast(PlannedStmt *plannedstmt, Plan *plan, Param **param)
{
ListCell *lc;
if (plan == NULL)
return false;
if (contain_fishy_cast((Node *) plan->qual, param))
return true;
if (qual_has_fishy_cast(plannedstmt, innerPlan(plan), param))
return true;
if (qual_has_fishy_cast(plannedstmt, outerPlan(plan), param))
return true;
foreach(lc, plan->initPlan)
{
SubPlan *subplan = (SubPlan *) lfirst(lc);
Plan *s_plan = exec_subplan_get_plan(plannedstmt, subplan);
if (qual_has_fishy_cast(plannedstmt, s_plan, param))
return true;
}
return false;
}
/*
* Common part of some expression based analyzes.
*
@@ -5163,6 +5261,7 @@ expr_get_desc(PLpgSQL_checkstate *cstate,
if (IsA(_stmt, PlannedStmt) &&_stmt->commandType == CMD_SELECT)
{
_plan = _stmt->planTree;
if (IsA(_plan, Result) &&list_length(_plan->targetlist) == 1)
{
tle = (TargetEntry *) linitial(_plan->targetlist);
@@ -5377,6 +5476,76 @@ prohibit_transaction_stmt(PLpgSQL_checkstate *cstate, PLpgSQL_expr *query)
ReleaseCachedPlan(cplan, true);
}
/*
* Raise a performance warning when plan hash fishy qual
*/
static void
check_fishy_qual(PLpgSQL_checkstate *cstate, PLpgSQL_expr *query)
{
CachedPlanSource *plansource = NULL;
SPIPlanPtr plan = query->plan;
CachedPlan *cplan;
List *stmt_list;
ListCell *lc;
if (plan == NULL || plan->magic != _SPI_PLAN_MAGIC)
elog(ERROR, "cached plan is not valid plan");
if (list_length(plan->plancache_list) != 1)
elog(ERROR, "plan is not single execution plan");
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
#if PG_VERSION_NUM >= 100000
cplan = GetCachedPlan(plansource, NULL, true, NULL);
#else
cplan = GetCachedPlan(plansource, NULL, true);
#endif
stmt_list = cplan->stmt_list;
foreach(lc, stmt_list)
{
Param *param;
#if PG_VERSION_NUM >= 100000
PlannedStmt *pstmt = (PlannedStmt *) lfirst(lc);
Plan *plan = NULL;
Assert(IsA(pstmt, PlannedStmt));
plan = pstmt->planTree;
#else
Node *pstmt = (Node *) lfirst(lc);
Plan *plan = NULL;
#endif
if (qual_has_fishy_cast(pstmt, plan, &param))
{
put_error(cstate,
ERRCODE_DATATYPE_MISMATCH, 0,
"implicit cast of attribute caused by different PLpgSQL variable type in WHERE clause",
"An index of some attribute cannot be used, when variable, used in predicate, has not right type like a attribute",
"Check a variable type - int versus numeric",
PLPGSQL_CHECK_WARNING_PERFORMANCE,
param->location,
query->query, NULL);
}
}
ReleaseCachedPlan(cplan, true);
}
/*
* returns refname of PLpgSQL_datum
*/
@@ -2078,3 +2078,46 @@ alter table testtable drop column b;
-- there is not possibility to enforce recompilation
-- before checking.
select * from plpgsql_check_function('test()');
drop function test();
-- issue #32
create table bigtable(id bigint, v varchar);
create or replace function test()
returns void as $$
declare
r record;
_id numeric;
begin
select * into r from bigtable where id = _id;
for r in select * from bigtable where _id = id
loop
end loop;
if (exists(select * from bigtable where id = _id)) then
end if;
end;
$$ language plpgsql;
select test();
-- should to show performance warnings
select * from plpgsql_check_function('test()', performance_warnings => true);
create or replace function test()
returns void as $$
declare
r record;
_id bigint;
begin
select * into r from bigtable where id = _id;
for r in select * from bigtable where _id = id
loop
end loop;
if (exists(select * from bigtable where id = _id)) then
end if;
end;
$$ language plpgsql;
-- there are not any performance issue now
select * from plpgsql_check_function('test()', performance_warnings => true);

0 comments on commit 68b899e

Please sign in to comment.