Hi Alena, Em seg., 10 de jul. de 2023 às 05:38, Alena Rybakina < lena.riback...@yandex.ru> escreveu:
> 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. 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; + } 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)) 3. Sorry about wrong tip about array_type, but if really necessary, better use it. + newa->element_typeid = scalar_type; + newa->array_typeid = array_type; 4. Is a good habit, call free last, to avoid somebody accidentally using it. + or_list = lappend(or_list, gentry->expr); + list_free(gentry->consts); + continue; 5. list_make1(makeString((char *) "=") Is an invariant? We can store in a variable and keep out the loop? Keep up the good work. best regards, Ranier Vilela