On 12/2/2024 17:51, Alena Rybakina wrote:
> On 12.02.2024 12:01, Andrei Lepikhov wrote:
>> On 12/2/2024 15:55, Alena Rybakina wrote:
>>> 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"?
IMO, it is a matter of taste. But if you are really confused, maybe it
will make understanding for someone else simpler. So, changed.
> 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))
I wonder if we should add here assertion, not NULL check. In what case
we could get NULL clause here? But added for surety.
> 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.
I don't. Feel free to provide counterexample.
--
regards,
Andrei Lepikhov
Postgres Professional