On 12.02.2024 12:01, Andrei Lepikhov wrote:
> On 12/2/2024 15:55, Alena Rybakina wrote:
>> Hi!
>>
>> I can't unnderstand this part of code:
>>
>> /* Time to generate index paths */
>>
>> MemSet(&clauseset, 0, sizeof(clauseset));
>> match_clauses_to_index(root, list_make1(rinfo1), index, &clauseset);
>>
>> As I understand it, match_clauses_to_index is necessary if you have a
>> RestrictInfo (rinfo1) variable, so maybe we should run it after the
>> make_restrictonfo procedure, otherwise calling it, I think, is useless.
> I think you must explain your note in more detail. Before this call,
> we already called make_restrictinfo() and built rinfo1, haven't we?
>
I got it, I think, I was confused by the “else“ block when we can
process the index, otherwise we move on to the next element.
I think maybe “else“ block of creating restrictinfo with the indexpaths
list creation should be moved to a separate function or just remove "else"?
I think we need to check that rinfo->clause is not empty, because if it
is we can miss calling build_paths_for_OR function. We should add it there:
restriction_is_saop_clause(RestrictInfo *restrictinfo)
{
if (IsA(restrictinfo->clause, ScalarArrayOpExpr))
...
By the way, I think we need to add a check that the clauseset is not
empty (if (!clauseset.nonempty)) otherwise we could get an error. The
same check I noticed in build_paths_for_OR function.
--
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company