public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 6/6] Ignore correlation for new BRIN opclasses 3+ messages / 3 participants [nested] [flat]
* [PATCH 6/6] Ignore correlation for new BRIN opclasses @ 2020-09-12 13:07 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw) The new BRIN opclasses (bloom and minmax-multi) are less sensitive to poorly correlated data, so just assume the data is perfectly correlated during costing. Author: Tomas Vondra <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin_bloom.c | 1 + src/backend/access/brin/brin_minmax_multi.c | 1 + src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++- src/include/access/brin_internal.h | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index 4494484b3b..46ffc068be 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -412,6 +412,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(BloomOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (BloomOpaque *) diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index c522bf2285..98382df3c7 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -1744,6 +1744,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(MinmaxMultiOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (MinmaxMultiOpaque *) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 52314d3aa1..0320d128f6 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -98,6 +98,7 @@ #include <math.h> #include "access/brin.h" +#include "access/brin_internal.h" #include "access/brin_page.h" #include "access/gin.h" #include "access/table.h" @@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, double minimalRanges; double estimatedRanges; double selec; - Relation indexRel; + Relation indexRel = NULL; + TupleDesc tupdesc = NULL; ListCell *l; VariableStatData vardata; @@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, */ indexRel = index_open(index->indexoid, NoLock); brinGetStats(indexRel, &statsData); + tupdesc = RelationGetDescr(indexRel); index_close(indexRel, NoLock); /* work out the actual number of ranges in the index */ @@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, { IndexClause *iclause = lfirst_node(IndexClause, l); AttrNumber attnum = index->indexkeys[iclause->indexcol]; + FmgrInfo *opcInfoFn; + BrinOpcInfo *opcInfo; + Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol); + bool ignore_correlation; + + opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO); + + opcInfo = (BrinOpcInfo *) + DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid)); + + ignore_correlation = opcInfo->oi_ignore_correlation; /* attempt to lookup stats in relation for this index column */ if (attnum != 0) @@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (sslot.nnumbers > 0) varCorrelation = Abs(sslot.numbers[0]); + if (ignore_correlation) + varCorrelation = 1.0; + if (varCorrelation > *indexCorrelation) *indexCorrelation = varCorrelation; diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index fdaff42722..5fbf8cf9c7 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -30,6 +30,9 @@ typedef struct BrinOpcInfo /* Regular processing of NULLs in BrinValues? */ bool oi_regular_nulls; + /* Ignore correlation during cost estimation */ + bool oi_ignore_correlation; + /* Opaque pointer for the opclass' private use */ void *oi_opaque; -- 2.26.2 --------------6A37408A2C42B5B56C3B3D49-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add backendType to PGPROC, replacing isRegularBackend @ 2026-02-03 21:04 Nathan Bossart <[email protected]> 2026-02-04 11:08 ` Re: Add backendType to PGPROC, replacing isRegularBackend Heikki Linnakangas <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Nathan Bossart @ 2026-02-03 21:04 UTC (permalink / raw) To: Heikki Linnakangas <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers On Tue, Feb 03, 2026 at 10:55:20PM +0200, Heikki Linnakangas wrote: > I propose a little refactoring, attached, to replace the "isRegularBackend" > field in PGPROC with full "backendType". > > Andres briefly suggested this a while back [1]: > > On Fri, 22 Nov 2024 at 22:13, Andres Freund <andres(at)anarazel(dot)de> > wrote: >> Or we could have a copy of the backend type in PGPROC. > > but we didn't follow up on that approach. I don't see why, it seems so much > simpler than what we ended up doing. Am I missing something? At a glance, it looks reasonable to me. I don't recall whether I explored this approach, but at the very least I'm unaware of any reason it wouldn't work. > @@ -684,7 +684,7 @@ InitAuxiliaryProcess(void) > MyProc->databaseId = InvalidOid; > MyProc->roleId = InvalidOid; > MyProc->tempNamespaceId = InvalidOid; > - MyProc->isRegularBackend = false; > + MyProc->backendType = B_INVALID; > MyProc->delayChkptFlags = 0; > MyProc->statusFlags = 0; > MyProc->lwWaiting = LW_WS_NOT_WAITING; Hm. So for auxiliary processes, this would always be unset? That appears to be alright for today's use-cases, but it could be a problem down the road. -- nathan ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add backendType to PGPROC, replacing isRegularBackend 2026-02-03 21:04 Re: Add backendType to PGPROC, replacing isRegularBackend Nathan Bossart <[email protected]> @ 2026-02-04 11:08 ` Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Heikki Linnakangas @ 2026-02-04 11:08 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers; Bertrand Drouvot <[email protected]>; Chao Li <[email protected]> On 03/02/2026 23:04, Nathan Bossart wrote: > On Tue, Feb 03, 2026 at 10:55:20PM +0200, Heikki Linnakangas wrote: >> I propose a little refactoring, attached, to replace the "isRegularBackend" >> field in PGPROC with full "backendType". >> >> Andres briefly suggested this a while back [1]: >> >> On Fri, 22 Nov 2024 at 22:13, Andres Freund <andres(at)anarazel(dot)de> >> wrote: >>> Or we could have a copy of the backend type in PGPROC. >> >> but we didn't follow up on that approach. I don't see why, it seems so much >> simpler than what we ended up doing. Am I missing something? > > At a glance, it looks reasonable to me. I don't recall whether I explored > this approach, but at the very least I'm unaware of any reason it wouldn't > work. Ok. >> @@ -684,7 +684,7 @@ InitAuxiliaryProcess(void) >> MyProc->databaseId = InvalidOid; >> MyProc->roleId = InvalidOid; >> MyProc->tempNamespaceId = InvalidOid; >> - MyProc->isRegularBackend = false; >> + MyProc->backendType = B_INVALID; >> MyProc->delayChkptFlags = 0; >> MyProc->statusFlags = 0; >> MyProc->lwWaiting = LW_WS_NOT_WAITING; > > Hm. So for auxiliary processes, this would always be unset? That appears > to be alright for today's use-cases, but it could be a problem down the > road. Right, that was just a silly mistake. Fixed that and pushed, thanks! - Heikki ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-02-04 11:08 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-12 13:07 [PATCH 6/6] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]> 2026-02-03 21:04 Re: Add backendType to PGPROC, replacing isRegularBackend Nathan Bossart <[email protected]> 2026-02-04 11:08 ` Re: Add backendType to PGPROC, replacing isRegularBackend Heikki Linnakangas <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox