Re: A little cosmetic to convert_VALUES_to_ANY()

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: A little cosmetic to convert_VALUES_to_ANY()
Date: 2025-08-04 04:40:13
Message-ID: CAHewXNnyPk8=2NRzkYRSYDiSjjYrZoMaGM2VpFkeYYkjm-qaVQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com> 于2025年8月4日周一 12:33写道:

> 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.
>

Agree, I will withdraw the patch.

>
> 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.
>

About this, you can check this commit
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=aadf7db66ef5a8a723eb3362e2c8b460738f1107
.
Add brace for better/more consistent style.

--
Thanks,
Tender Wang

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2025-08-04 04:52:20 Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX
Previous Message Michael Paquier 2025-08-04 04:39:10 Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX