Hi Alexander,
> I think we still need to check `InvalidOid` because execIndexing.c:~800
> initializes scan keys ('ScanKeyEntryInitialize') with `InvalidOid` as the
> subtype. Running the 'not_equal', 'partitions' and 'without_overlaps' tests
> confirms that 'InvalidOid' must be handled. I can add the check again in the
> switch. (attaching patch)
You are completely right, I missed this while reading the source code.
> I completely agree, your patch makes it much cleaner.
> I applied your patch (with the 'InvalidOid' fix) on top of mine and ran all
> the
> tests and benchmarks that I prepared previously. [...]
I'm glad you like the suggestion, and thank you for the pgindent
fixes that I missed.
Since this implementation now replaces the initial one in 0001, it
might be worth squashing 0001 and 0004 (and the 0005 fixes) into a
single 0001 patch, unless you want to keep the history of this work
in the patch series.
> I added one more commit that adds a small description of this fix in the docs.
This patch also looks good to me, although I may not be the best
person to review doc changes.
One last tiny thing: 0001 adds int_crosstype to the regress tests
but the test file is only added in 0002, so 0001 on its own fails
`make check`. Might be worth moving the Makefile/meson.build
registration into 0002 so each patch passes its own tests.
I think that one additional thing needed before it's ready would be
to add commit messages to each patch.
Other than that it looks good, I don't have any additional code
comments.
Best,
Maxime
