On 11.07.2023 16:29, Ranier Vilela wrote:
Hi!
On 10.07.2023 15:15, Ranier Vilela wrote:
Hi Alena,
I agreed with the changes. Thank you for your work.
I updated patch and added you to the authors.
I specified Ranier Vilela as a reviewer.
Is a good habit when post a new version of the patch, name it v1, v2, v3,etc.
Makes it easy to follow development and references on the thread.
Sorry, I fixed it.
Regarding the last patch.
1. I think that variable const_is_left is not necessary.
You can stick with:
+ if (IsA(get_leftop(orqual), Const))
+ nconst_expr =get_rightop(orqual);
+ const_expr = get_leftop(orqual) ;
+ else if (IsA(get_rightop(orqual), Const))
+ nconst_expr =get_leftop(orqual);
+ const_expr = get_rightop(orqual) ;
+ else
+ {
+ or_list = lappend(or_list, orqual);
+ continue;
+ }
Agreed.
You missed in removing the declaration
- bool const_is_left = true;
Yes, thank you. I fixed it.
.
2. Test scalar_type != RECORDOID is more cheaper,
mainly if OidIsValid were a function, we knows that is a macro.
+ if (scalar_type != RECORDOID && OidIsValid(scalar_type))
Is it safe? Maybe we should first make sure that it can be checked on RECORDOID at all?
Yes it's safe, because && connector.
But you can leave as is in v5.
Added it.
--
Regards,
Alena Rybakina
Postgres Professional