Re: A little cosmetic to convert_VALUES_to_ANY()

On Sun, Jul 27, 2025 at 12:53 AM Tender Wang <tndrwang@gmail.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.