Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Tue, Jan 14, 2020 at 9:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> One thing I'm still not happy about is the comment in
>> collectMatchesForHeapRow. v12 failed to touch that at all, so I tried to
>> fill it in, but I'm not sure if my explanation is good.
> I've tried to rephrase this comment making it better from my point of
> view. It's hard for me to be sure about this, since I'm not native
> English speaker. I'd like you to take a look on it.
Yeah, that's not great as-is. Maybe like
+ * All scan keys except excludeOnly require at least one entry to match.
+ * excludeOnly keys are an exception, because their implied
+ * GIN_CAT_EMPTY_QUERY scanEntry always matches. So return "true"
+ * if all non-excludeOnly scan keys have at least one match.
>> Also, if we know
>> that excludeOnly keys are going to be ignored, can we save any work in
>> the main loop of that function?
> It doesn't look so for me. We still need to collect matches for
> consistent function call afterwards.
Ah, right.
> I also had concerns about how excludeOnly keys work with lossy pages.
> I didn't find exact error. But I've added code, which skips
> excludeOnly keys checks for lossy pages. They aren't going to exclude
> any lossy page anyway. So, we can save some resources by skipping
> this.
Hmm ... yeah, these test cases are not large enough to exercise any
lossy-page cases, are they? I doubt we should try to make a new regression
test that is that big. (But if there is one already, maybe we could add
more test queries with it, instead of creating whole new tables?)
regards, tom lane