Hi, On Mon, 22 Jun 2026 at 14:44, Andrei Lepikhov <[email protected]> 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". Otherwise LGTM. Will mark it ready for committer. Regards, Ayush
