On Mon, 22 Jun 2026 at 14:44, Andrei Lepikhov <lepihov@gmail.com> wrote:
On 19/06/2026 15:28, Nishant Sharma wrote: > Thanks for the v0 patch set! > > The topic is new to me. I thought I would review it to see if I could learn > something. > > Code changes are small, simple and clean.
Thanks for your interest!
> > Here are my comments: > 1. Should we add a test case that confirms the comment: "this never appears in > deparsed views, rules, or other stored expressions". Also, what about multirange > tests?
Ok, done.
> 2. I see that the word "hashed" is added for all valid hashfuncid in the plan, > which implies that hash is used instead of array for those cases. If this is > being done, why not explicitly include "linear" or "array" for non-valid > hashfuncid instead of keeping "" empty, given that the goal is to provide more > information? I saw too many differences in existing tests and also in the > upgrade test in make check world. So, I think that's not a good idea. Here, we just follow the hashed SubPlan approach. I think we don't need anything else here.
Thanks for the patch!
Post test suite, I spot-checked a hashed SAOP as an index condition and x = ANY (ARRAY(SELECT ...)); both behave correctly.
One nit in the comment of a test case: "Check that 'hashed' is actually bubbles up...", we may drop the "is".
