Re: A little cosmetic to convert_VALUES_to_ANY()

On Sun, Jul 27, 2025 at 12:53 AM Tender Wang <tndrwang(at)gmail(dot)com> wrote:

>
> We can merge the two scans into one. This can reduce the time complexity
> from 2O(n) to O(n).
>

This seems like an unusual usage of big-O notation. Always saw the
meaningful pieces inside the O, and in general I thought O(n) =~ O(xn) for
all x, but especially if x is small.

The point made by Chao Li, that by using a loop guard we avoid performing
expression evaluation, seems like a reasonable and sufficient non-data
supported reason for having the code structured this way. I'd at least
want a more data-driven reason to try and change it in such a simple way
against the read/write boundary that presently exists.

I was pondering whether contain_volatile_functions could/should be taught
to also detect whether every expression it sees resolves to a constant...
contain_volatile_functions((Node *) rte->values_lists, &has_non_constants)

But got a little ways down the call stack and hit the "walker" macro and
decided to stop...

> Also, add a brace for better/more consistent style in the attached patch.
>

Should try and avoid oh-by-the-way fixes like this in behavior patches.

This specific one also doesn't seem warranted. Or, at least, the original
style is indeed de facto acceptable; if-blocks containing a single
executing statement do not get braces but rely on indentation alone. I
don't think that every other if-block in the region being multi-statement
supports an exception.

David J.