Re: BUG #17360: array_to_string should be immutable instead of stable

"David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> On Mon, Jan 10, 2022 at 7:54 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Nope. It invokes an arbitrary datatype I/O function,
>> which might only be stable. As an example:

> That feels wrong. It's not like we are passing the "now()" function to the
> function and invoking it later. So far as array_to_string is concerned it
> is being given a literal value.

now() is not the problem; it's the datatype output function that's
the problem. I was perhaps being too thrifty with keystrokes in
my example, so here's another one:

regression=# show timezone;
TimeZone
------------------
America/New_York
(1 row)

regression=# select '{2022-01-10 00:00-05}'::timestamptz[];
timestamptz
----------------------------
{"2022-01-10 00:00:00-05"}
(1 row)

regression=# select array_to_string('{2022-01-10 00:00-05}'::timestamptz[], ',');
array_to_string
------------------------
2022-01-10 00:00:00-05
(1 row)

regression=# set timezone = UTC;
SET
regression=# select '{2022-01-10 00:00-05}'::timestamptz[];
timestamptz
----------------------------
{"2022-01-10 05:00:00+00"}
(1 row)

regression=# select array_to_string('{2022-01-10 00:00-05}'::timestamptz[], ',');
array_to_string
------------------------
2022-01-10 05:00:00+00
(1 row)

Now do you see the issue? The input datum is identical in all four
queries, but the resulting strings are not, so these functions
cannot be considered immutable.

> In short, that doesn't make sense. The volatility level of a function is
> only determined by the implementation code of said function.

array_to_string is invoking timestamptz_out along the way to
creating its result. Although array_to_string's own behavior
is immutable, timestamptz_out's is not.

> The function
> invoking expression volatility level depends upon the most volatile
> behavior of all functions used in the expression. That we should be doing
> if we aren't already.

The core of the difficulty is that although timestamptz_out
is getting called, that's nowhere visible in the parse tree.
I suppose we could decide that it's illegal to allow
array_to_string() or format() to exist, but I don't think
anybody will like that answer.

I did just have a thought about this though --- now that we've
invented planner support functions [1], maybe we could define
a support function request that is "tell me the true volatility
of this function call". Then array_to_string() could have a
support function that looks at the output function for its
input array's element type, and format()'s could determine the
most volatile of the output functions of any of its inputs, etc.

regards, tom lane

[1] https://www.postgresql.org/docs/devel/xfunc-optimization.html