On 2018/03/06 18:46, Emre Hasegeli wrote: >> Patch teaches it to ignore nulls when it's known that the operator being >> used is strict. It is harmless and has the benefit that constraint >> exclusion gives an answer that is consistent with what actually running >> such a qual against a table's rows would do. > > Yes, I understood that. I just meant that I don't know if it is the > best way to skip NULLs inside "next_fn". Maybe the caller of the > "next_fn"s should skip them. Anyway, the committer can judge this > better.
I think putting that inside those next_fn functions tends to centralize the logic and would run only only for the intended cases. >> Yeah. Rearranged the code to fix that. > > This version looks correct to me. > >> + state->next = (state->next != NULL) ? lnext(state->next) : NULL; >> + node = (state->next != NULL) ? lfirst(state->next) : NULL; > > I think it is unnecessary to check for (state->next != NULL) two > times. We can put those in a single if. Hmm, state->next refers to two different pointer values on line 1 and line 2. It may end up being set to NULL on line 1. Am I missing something? Thanks, Amit
