I noticed by accident that there are some cases where the planner fails
to inline the SQL functions that underlie the || operator for text vs
non-text cases. The reason is that these functions are marked
immutable, but their expansion involves a coercion to text that might
not be immutable. The planner will not inline a function if the
resulting expression would be more volatile than the function claims
itself to be, because sometimes the point of such a function is to hide
the expression's volatility. In this case, however, we don't want to
hide the true nature of the expression, and we definitely don't want
to pay the performance price of calling a SQL function. That price
is pretty significant, eg on a rather slow machine I get
regression=# select count(localtimestamp || i::text) from generate_series(1,100000) i;count
--------100000
(1 row)
Time: 12512.624 ms
regression=# update pg_proc set provolatile = 'v' where oid = 2004;
UPDATE 1
Time: 7.104 ms
regression=# select count(localtimestamp || i::text) from generate_series(1,100000) i;count
--------100000
(1 row)
Time: 4967.086 ms
so the added overhead more than doubles the cost of this case.
There's also a possibility of an outright wrong behavior, since the
immutable marking will allow the concatenation of two constants to
be folded to a constant in contexts where perhaps it shouldn't be.
Continuing the above example, 'now'::timestamp || 'foo' will be folded
to a constant on sight, which is wrong because the coercion to text
depends on DateStyle and ought to react to a later change in DateStyle.
So I think that labeling textanycat/anytextcat as immutable was a
thinko, and we instead ought to label them volatile so that the planner
can inline them no matter what the behavior of the underlying text cast
is.
Is it reasonable to fix this now, and if so should I bump catversion
or leave it alone? My own preference is to fix it in pg_proc.h but
not touch catversion; but you could argue that different ways.
regards, tom lane
