Re: A little cosmetic to convert_VALUES_to_ANY() - Mailing list pgsql-hackers

From Tender Wang
Subject Re: A little cosmetic to convert_VALUES_to_ANY()
Date
Msg-id CAHewXNkETeYSA85tfdHamz_hA8Ynr-Haxom=AGcofWzr7rfJRw@mail.gmail.com
Whole thread Raw
In response to Re: A little cosmetic to convert_VALUES_to_ANY()  (Chao Li <li.evan.chao@gmail.com>)
List pgsql-hackers


Chao Li <li.evan.chao@gmail.com> 于2025年8月4日周一 11:11写道:
I tested this patch locally and it works for me.

However I doubt if it really improve performance. The original code "contain_volatile_functions((Node *) rte->values_lists)" recursively work through rte-values_list, and this patch move contain_volatile_functions() into the for loop, and checks against every item of rte->values_list. As entries of values_list could just be Const, contain_volatile_functions() will then return immediately, so that this changes reduces total calls to contain_volatile_functions(). From this perspective, this patch may improve the performance.

On the other side, let's say a case where a values_list contains 10 items, and the last item is volatile. With the original code, it won't enter the for loop, nothing will be built; with this patch, operations have been performed on the first 9 items, but eventually it returns NULL when hit the last item. So in this case, performance could be worse.

Yes, in this case, the original code scans the values_list, finds the volatile functions, and returns from convert_VALUES_to_ANY(). 
My patch will perform unnecessary convert_testexpr and eval_const_expressions work. 

Overall, I am afraid the burden could beat the gain.

It's not easy to evaluate performance gain.   
I'm not sure it's worth doing the change. If someone doesn't like the patch. I will withdraw it.


The new status of this patch is: Waiting on Author
Thanks for your review.

Actually, the purpose of convert_VALUES_to_ANY() is only to convert the sublink that includes the Values expression to SAOP.
I have a question: Can we move the convert_VALUES_to_ANY() to another place? For example:preprocess_qual_conditions().
In pull_up_sublinks_jointree_recurse(), what we want to do is to pull up the sublinks.

--
Thanks,
Tender Wang

pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: implement CAST(expr AS type FORMAT 'template')
Next
From: Ashutosh Bapat
Date:
Subject: Re: Dropping publication breaks logical replication