On 2021-Mar-03, Tomas Vondra wrote: > 1) The 0001 patch allows passing of all scan keys to BRIN opclasses, > which is needed for the minmax-multi to work. But it also modifies the > existing opclasses (minmax and inclusion) to do this - but for those > opclasses it does not make much difference, I think. Initially this was > done because the patch did this for all opclasses, but then we added the > detection based on number of parameters. So I wonder if we should just > remove this, to make the patch a bit smaller. It'll also test the other > code path (for opclasses without the last parameter).
I think it makes sense to just do them all in one pass. I think trying to keep the old way working just because it's how it was working previously does not have much benefit. I don't think we care about the *patch* being small as much as the resulting *code* being as simple as possible (but no simpler). > 2) This needs bsearch_arg, but we only have that defined/used in > extended_statistics_internal.h - it seems silly to include that here, as > this has nothing to do with extended stats, so I simply added another > prototype (which gets linked correctly). But, I suppose a better way > would be to define our "portable" variant pg_bsearch_arg, next to > pg_qsort etc. Yeah, I think it makes sense to move bsearch_arg to a place where it's more generally accesible (src/port/bsearch_arg.c I suppose), and make both places use that. -- Álvaro Herrera Valdivia, Chile
