Skip to content

Commit 0f9f12c

Browse files
okbobokbob
committed
ensure tracking constants only for scalar variables
1 parent c6cb7d5 commit 0f9f12c

File tree

6 files changed

+143
-27
lines changed

6 files changed

+143
-27
lines changed

‎expected/plpgsql_check_active.out‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,27 @@ select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warn
17151715
foreach_array_loop | | | 00000 | routine is marked as STABLE, should be IMMUTABLE | | When you fix this issue, please, recheck other functions that uses this function. | performance | | |
17161716
(2 rows)
17171717

1718+
create or replace function foreach_array_loop()
1719+
returns void as
1720+
$body$
1721+
declare
1722+
arr date[];
1723+
el int;
1724+
begin
1725+
-- expression is not constant (else this check fails due tracking constant)
1726+
arr := array['2014-01-01','2015-01-01','2016-01-01', current_date]::date[];
1727+
foreach el in array arr loop
1728+
raise notice '%', el;
1729+
end loop;
1730+
end;
1731+
$body$
1732+
language 'plpgsql' stable;
1733+
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
1734+
functionid | lineno | statement | sqlstate | message | detail | hint | level | position | query | context
1735+
--------------------+--------+--------------------+----------+------------------------------------------------+-------------------------------------+----------------------------------------------------------------------------+---------+----------+-------+-----------------------
1736+
foreach_array_loop | 8 | FOREACH over array | 42804 | target type is different type than source type | cast "date" value to "integer" type | There are no possible explicit coercion between those types, possibly bug! | warning | | | at FOREACH over array
1737+
(1 row)
1738+
17181739
create or replace function foreach_array_loop()
17191740
returns void as
17201741
$body$

‎expected/plpgsql_check_active_2.out‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,27 @@ select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warn
17151715
foreach_array_loop | | | 00000 | routine is marked as STABLE, should be IMMUTABLE | | When you fix this issue, please, recheck other functions that uses this function. | performance | | |
17161716
(2 rows)
17171717

1718+
create or replace function foreach_array_loop()
1719+
returns void as
1720+
$body$
1721+
declare
1722+
arr date[];
1723+
el int;
1724+
begin
1725+
-- expression is not constant (else this check fails due tracking constant)
1726+
arr := array['2014-01-01','2015-01-01','2016-01-01', current_date]::date[];
1727+
foreach el in array arr loop
1728+
raise notice '%', el;
1729+
end loop;
1730+
end;
1731+
$body$
1732+
language 'plpgsql' stable;
1733+
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
1734+
functionid | lineno | statement | sqlstate | message | detail | hint | level | position | query | context
1735+
--------------------+--------+--------------------+----------+------------------------------------------------+-------------------------------------+----------------------------------------------------------------------------+---------+----------+-------+-----------------------
1736+
foreach_array_loop | 8 | FOREACH over array | 42804 | target type is different type than source type | cast "date" value to "integer" type | There are no possible explicit coercion between those types, possibly bug! | warning | | | at FOREACH over array
1737+
(1 row)
1738+
17181739
create or replace function foreach_array_loop()
17191740
returns void as
17201741
$body$

‎expected/plpgsql_check_active_3.out‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,27 @@ select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warn
17151715
foreach_array_loop | | | 00000 | routine is marked as STABLE, should be IMMUTABLE | | When you fix this issue, please, recheck other functions that uses this function. | performance | | |
17161716
(2 rows)
17171717

1718+
create or replace function foreach_array_loop()
1719+
returns void as
1720+
$body$
1721+
declare
1722+
arr date[];
1723+
el int;
1724+
begin
1725+
-- expression is not constant (else this check fails due tracking constant)
1726+
arr := array['2014-01-01','2015-01-01','2016-01-01', current_date]::date[];
1727+
foreach el in array arr loop
1728+
raise notice '%', el;
1729+
end loop;
1730+
end;
1731+
$body$
1732+
language 'plpgsql' stable;
1733+
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
1734+
functionid | lineno | statement | sqlstate | message | detail | hint | level | position | query | context
1735+
--------------------+--------+--------------------+----------+------------------------------------------------+-------------------------------------+----------------------------------------------------------------------------+---------+----------+-------+-----------------------
1736+
foreach_array_loop | 8 | FOREACH over array | 42804 | target type is different type than source type | cast "date" value to "integer" type | There are no possible explicit coercion between those types, possibly bug! | warning | | | at FOREACH over array
1737+
(1 row)
1738+
17181739
create or replace function foreach_array_loop()
17191740
returns void as
17201741
$body$

‎expected/plpgsql_check_active_4.out‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,27 @@ select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warn
17151715
foreach_array_loop | | | 00000 | routine is marked as STABLE, should be IMMUTABLE | | When you fix this issue, please, recheck other functions that uses this function. | performance | | |
17161716
(2 rows)
17171717

1718+
create or replace function foreach_array_loop()
1719+
returns void as
1720+
$body$
1721+
declare
1722+
arr date[];
1723+
el int;
1724+
begin
1725+
-- expression is not constant (else this check fails due tracking constant)
1726+
arr := array['2014-01-01','2015-01-01','2016-01-01', current_date]::date[];
1727+
foreach el in array arr loop
1728+
raise notice '%', el;
1729+
end loop;
1730+
end;
1731+
$body$
1732+
language 'plpgsql' stable;
1733+
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
1734+
functionid | lineno | statement | sqlstate | message | detail | hint | level | position | query | context
1735+
--------------------+--------+--------------------+----------+------------------------------------------------+-------------------------------------+----------------------------------------------------------------------------+---------+----------+-------+-----------------------
1736+
foreach_array_loop | 8 | FOREACH over array | 42804 | target type is different type than source type | cast "date" value to "integer" type | There are no possible explicit coercion between those types, possibly bug! | warning | | | at FOREACH over array
1737+
(1 row)
1738+
17181739
create or replace function foreach_array_loop()
17191740
returns void as
17201741
$body$

‎sql/plpgsql_check_active.sql‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,24 @@ language 'plpgsql' stable;
11541154

11551155
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
11561156

1157+
create or replace function foreach_array_loop()
1158+
returns void as
1159+
$body$
1160+
declare
1161+
arr date[];
1162+
el int;
1163+
begin
1164+
-- expression is not constant (else this check fails due tracking constant)
1165+
arr := array['2014-01-01','2015-01-01','2016-01-01', current_date]::date[];
1166+
foreach el in array arr loop
1167+
raise notice '%', el;
1168+
end loop;
1169+
end;
1170+
$body$
1171+
language 'plpgsql' stable;
1172+
1173+
select * from plpgsql_check_function_tb('foreach_array_loop()', performance_warnings := true);
1174+
11571175
create or replace function foreach_array_loop()
11581176
returns void as
11591177
$body$

‎src/check_expr.c‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,42 +1616,56 @@ plpgsql_check_expr_as_rvalue(PLpgSQL_checkstate *cstate, PLpgSQL_expr *expr,
16161616
}
16171617
else if (cstate->cinfo->constants_tracing && targetdno != -1)
16181618
{
1619-
char *str;
1619+
PLpgSQL_var *var = (PLpgSQL_var *) cstate->estate->datums[targetdno];
16201620

1621-
str = plpgsql_check_expr_get_string(cstate, expr, NULL);
1622-
if (str)
1621+
/*
1622+
* Trace constants only when target is really scalar.
1623+
* The scalar types allows simple IO casting, but composites with different
1624+
* number of fields doesn't allow it. There are difference between PL/pgSQL
1625+
* assignment, that is not sensitive on different numbers of fields between
1626+
* composite target and composite source, and common composite casts (that
1627+
* doesn't allow to cast composites when source and target has different
1628+
* natts.
1629+
*/
1630+
if (var->dtype == PLPGSQL_DTYPE_VAR && var->datatype->ttype == PLPGSQL_TTYPE_SCALAR)
16231631
{
1624-
PLpgSQL_stmt_stack_item *current = cstate->top_stmt_stack;
1625-
MemoryContext oldcxt = MemoryContextSwitchTo(cstate->check_cxt);
1626-
char *prev_val;
1632+
char *str;
16271633

1628-
Assert(cstate->top_stmt_stack);
1634+
str = plpgsql_check_expr_get_string(cstate, expr, NULL);
1635+
if (str)
1636+
{
1637+
PLpgSQL_stmt_stack_item *current = cstate->top_stmt_stack;
1638+
MemoryContext oldcxt = MemoryContextSwitchTo(cstate->check_cxt);
1639+
char *prev_val;
16291640

1630-
if (!cstate->strconstvars)
1631-
cstate->strconstvars = palloc0(sizeof(char *) * cstate->estate->ndatums);
1641+
Assert(cstate->top_stmt_stack);
16321642

1633-
/*
1634-
* We need to do copy string first. There is an possibility to
1635-
* self reference, and then we need to first copy, and after
1636-
* that free.
1637-
*/
1638-
prev_val = cstate->strconstvars[targetdno];
1639-
cstate->strconstvars[targetdno] = pstrdup(str);
1643+
if (!cstate->strconstvars)
1644+
cstate->strconstvars = palloc0(sizeof(char *) * cstate->estate->ndatums);
16401645

1641-
if (prev_val)
1642-
pfree(prev_val);
1646+
/*
1647+
* We need to do copy string first. There is an possibility to
1648+
* self reference, and then we need to first copy, and after
1649+
* that free.
1650+
*/
1651+
prev_val = cstate->strconstvars[targetdno];
1652+
cstate->strconstvars[targetdno] = pstrdup(str);
16431653

1644-
current->invalidate_strconstvars = bms_add_member(current->invalidate_strconstvars, targetdno);
1654+
if (prev_val)
1655+
pfree(prev_val);
16451656

1646-
MemoryContextSwitchTo(oldcxt);
1647-
}
1648-
else
1649-
{
1650-
/* the assigned value is not constant, reset current */
1651-
if (cstate->strconstvars && cstate->strconstvars[targetdno])
1657+
current->invalidate_strconstvars = bms_add_member(current->invalidate_strconstvars, targetdno);
1658+
1659+
MemoryContextSwitchTo(oldcxt);
1660+
}
1661+
else
16521662
{
1653-
pfree(cstate->strconstvars[targetdno]);
1654-
cstate->strconstvars[targetdno] = NULL;
1663+
/* the assigned value is not constant, reset current */
1664+
if (cstate->strconstvars && cstate->strconstvars[targetdno])
1665+
{
1666+
pfree(cstate->strconstvars[targetdno]);
1667+
cstate->strconstvars[targetdno] = NULL;
1668+
}
16551669
}
16561670
}
16571671
}

0 commit comments

Comments
 (0)