public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
Subject: [PATCH 05/15] Maybe better than looping twice. For partitioned tables, create only "inherited" stats_ext.
Date: Wed, 3 Nov 2021 20:08:42 -0500
---
src/backend/commands/statscmds.c | 63 +++++---
src/backend/optimizer/util/plancat.c | 227 ++++++++++++++-------------
2 files changed, 151 insertions(+), 139 deletions(-)
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 4395d878c7..89e9ad5843 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -45,6 +45,7 @@
static char *ChooseExtendedStatisticName(const char *name1, const char *name2,
const char *label, Oid namespaceid);
static char *ChooseExtendedStatisticNameAddition(List *exprs);
+static void RemoveStatisticsByIdWorker(Relation pg_statistic_ext, Oid statsOid, bool inh);
/* qsort comparator for the attnums in CreateStatistics */
@@ -524,8 +525,9 @@ CreateStatistics(CreateStatsStmt *stmt)
datavalues[Anum_pg_statistic_ext_data_stxoid - 1] = ObjectIdGetDatum(statoid);
- /* create only the "stxdinherit=false", because that always exists */
- datavalues[Anum_pg_statistic_ext_data_stxdinherit - 1] = ObjectIdGetDatum(false);
+ /* create "stxdinherit=false", because that always exists, except for
+ * partitioned tables, for which that's meaningless */
+ datavalues[Anum_pg_statistic_ext_data_stxdinherit - 1] = BoolGetDatum(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
/* no statistics built yet */
datanulls[Anum_pg_statistic_ext_data_stxdndistinct - 1] = true;
@@ -729,30 +731,7 @@ RemoveStatisticsById(Oid statsOid)
HeapTuple tup;
Form_pg_statistic_ext statext;
Oid relid;
- int inh;
-
- /*
- * First delete the pg_statistic_ext_data tuple holding the actual
- * statistical data.
- */
- relation = table_open(StatisticExtDataRelationId, RowExclusiveLock);
-
- /* hack to delete both stxdinherit = true/false */
- for (inh = 0; inh <= 1; inh++)
- {
- tup = SearchSysCache2(STATEXTDATASTXOID, ObjectIdGetDatum(statsOid),
- BoolGetDatum(inh));
-
- if (!HeapTupleIsValid(tup)) /* should not happen */
- // elog(ERROR, "cache lookup failed for statistics data %u", statsOid);
- continue;
-
- CatalogTupleDelete(relation, &tup->t_self);
-
- ReleaseSysCache(tup);
- }
-
- table_close(relation, RowExclusiveLock);
+ char relkind;
/*
* Delete the pg_statistic_ext tuple. Also send out a cache inval on the
@@ -767,6 +746,7 @@ RemoveStatisticsById(Oid statsOid)
statext = (Form_pg_statistic_ext) GETSTRUCT(tup);
relid = statext->stxrelid;
+ relkind = get_rel_relkind(relid);
CacheInvalidateRelcacheByRelid(relid);
@@ -775,6 +755,37 @@ RemoveStatisticsById(Oid statsOid)
ReleaseSysCache(tup);
table_close(relation, RowExclusiveLock);
+
+ /*
+ * Delete the pg_statistic_ext_data tuples holding the actual
+ * statistical data.
+ */
+ relation = table_open(StatisticExtDataRelationId, RowExclusiveLock);
+
+ if (relkind != RELKIND_PARTITIONED_TABLE)
+ RemoveStatisticsByIdWorker(relation, statsOid, false);
+ RemoveStatisticsByIdWorker(relation, statsOid, true);
+
+ table_close(relation, RowExclusiveLock);
+}
+
+void
+RemoveStatisticsByIdWorker(Relation pg_statistic_ext, Oid statsOid, bool inh)
+{
+ HeapTuple tup;
+ tup = SearchSysCache2(STATEXTDATASTXOID, ObjectIdGetDatum(statsOid),
+ BoolGetDatum(inh));
+
+ /* should not happen, .. */
+ if (!HeapTupleIsValid(tup))
+ {
+ if (!inh)
+ elog(ERROR, "cache lookup failed for statistics data %u inh %d", statsOid, inh);
+ return;
+ }
+
+ CatalogTupleDelete(pg_statistic_ext, &tup->t_self);
+ ReleaseSysCache(tup);
}
/*
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 154d48a330..b1d8847529 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -81,6 +81,9 @@ static void set_baserel_partition_key_exprs(Relation relation,
static void set_baserel_partition_constraint(Relation relation,
RelOptInfo *rel);
+static void get_relation_statistics_worker(RelOptInfo *rel, List **stainfos,
+ Oid statOid, HeapTuple htup, Form_pg_statistic_ext staForm, Index varno,
+ bool inh);
/*
* get_relation_info -
@@ -1301,7 +1304,6 @@ get_relation_constraints(PlannerInfo *root,
static List *
get_relation_statistics(RelOptInfo *rel, Relation relation)
{
- Index varno = rel->relid;
List *statoidlist;
List *stainfos = NIL;
ListCell *l;
@@ -1312,150 +1314,149 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
{
Oid statOid = lfirst_oid(l);
Form_pg_statistic_ext staForm;
- Form_pg_statistic_ext_data dataForm;
HeapTuple htup;
- HeapTuple dtup;
- Bitmapset *keys = NULL;
- List *exprs = NIL;
- int i;
- int inh;
htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statOid));
if (!HeapTupleIsValid(htup))
elog(ERROR, "cache lookup failed for statistics object %u", statOid);
staForm = (Form_pg_statistic_ext) GETSTRUCT(htup);
- /*
- * Hack to load stats with stxdinherit true/false - there should be
- * a better way to do this, I guess.
- */
- for (inh = 0; inh <= 1; inh++)
- {
- dtup = SearchSysCache2(STATEXTDATASTXOID,
- ObjectIdGetDatum(statOid), BoolGetDatum((bool) inh));
- if (!HeapTupleIsValid(dtup))
- continue;
+ get_relation_statistics_worker(rel, &stainfos, statOid, htup, staForm, rel->relid, false);
+ get_relation_statistics_worker(rel, &stainfos, statOid, htup, staForm, rel->relid, true);
- dataForm = (Form_pg_statistic_ext_data) GETSTRUCT(dtup);
+ ReleaseSysCache(htup);
+ }
- /*
- * First, build the array of columns covered. This is ultimately
- * wasted if no stats within the object have actually been built, but
- * it doesn't seem worth troubling over that case.
- */
- for (i = 0; i < staForm->stxkeys.dim1; i++)
- keys = bms_add_member(keys, staForm->stxkeys.values[i]);
+ list_free(statoidlist);
- /*
- * Preprocess expressions (if any). We read the expressions, run them
- * through eval_const_expressions, and fix the varnos.
- */
- {
- bool isnull;
- Datum datum;
+ return stainfos;
+}
- /* decode expression (if any) */
- datum = SysCacheGetAttr(STATEXTOID, htup,
- Anum_pg_statistic_ext_stxexprs, &isnull);
+static void
+get_relation_statistics_worker(RelOptInfo *rel, List **stainfos, Oid statOid, HeapTuple htup, Form_pg_statistic_ext staForm, Index varno, bool inh)
+{
+ Form_pg_statistic_ext_data dataForm;
+ HeapTuple dtup;
+ Bitmapset *keys = NULL;
+ List *exprs = NIL;
+
+ dtup = SearchSysCache2(STATEXTDATASTXOID,
+ ObjectIdGetDatum(statOid), BoolGetDatum(inh));
+ if (!HeapTupleIsValid(dtup))
+ return;
- if (!isnull)
- {
- char *exprsString;
+ dataForm = (Form_pg_statistic_ext_data) GETSTRUCT(dtup);
- exprsString = TextDatumGetCString(datum);
- exprs = (List *) stringToNode(exprsString);
- pfree(exprsString);
+ /*
+ * First, build the array of columns covered. This is ultimately
+ * wasted if no stats within the object have actually been built, but
+ * it doesn't seem worth troubling over that case.
+ */
+ for (int i = 0; i < staForm->stxkeys.dim1; i++)
+ keys = bms_add_member(keys, staForm->stxkeys.values[i]);
- /*
- * Run the expressions through eval_const_expressions. This is
- * not just an optimization, but is necessary, because the
- * planner will be comparing them to similarly-processed qual
- * clauses, and may fail to detect valid matches without this.
- * We must not use canonicalize_qual, however, since these
- * aren't qual expressions.
- */
- exprs = (List *) eval_const_expressions(NULL, (Node *) exprs);
+ /*
+ * Preprocess expressions (if any). We read the expressions, run them
+ * through eval_const_expressions, and fix the varnos.
+ */
+ {
+ bool isnull;
+ Datum datum;
- /* May as well fix opfuncids too */
- fix_opfuncids((Node *) exprs);
+ /* decode expression (if any) */
+ datum = SysCacheGetAttr(STATEXTOID, htup,
+ Anum_pg_statistic_ext_stxexprs, &isnull);
- /*
- * Modify the copies we obtain from the relcache to have the
- * correct varno for the parent relation, so that they match
- * up correctly against qual clauses.
- */
- if (varno != 1)
- ChangeVarNodes((Node *) exprs, 1, varno, 0);
- }
- }
-
- /* add one StatisticExtInfo for each kind built */
- if (statext_is_kind_built(dtup, STATS_EXT_NDISTINCT))
- {
- StatisticExtInfo *info = makeNode(StatisticExtInfo);
+ if (!isnull)
+ {
+ char *exprsString;
- info->statOid = statOid;
- info->inherit = dataForm->stxdinherit;
- info->rel = rel;
- info->kind = STATS_EXT_NDISTINCT;
- info->keys = bms_copy(keys);
- info->exprs = exprs;
+ exprsString = TextDatumGetCString(datum);
+ exprs = (List *) stringToNode(exprsString);
+ pfree(exprsString);
- stainfos = lappend(stainfos, info);
- }
+ /*
+ * Run the expressions through eval_const_expressions. This is
+ * not just an optimization, but is necessary, because the
+ * planner will be comparing them to similarly-processed qual
+ * clauses, and may fail to detect valid matches without this.
+ * We must not use canonicalize_qual, however, since these
+ * aren't qual expressions.
+ */
+ exprs = (List *) eval_const_expressions(NULL, (Node *) exprs);
- if (statext_is_kind_built(dtup, STATS_EXT_DEPENDENCIES))
- {
- StatisticExtInfo *info = makeNode(StatisticExtInfo);
+ /* May as well fix opfuncids too */
+ fix_opfuncids((Node *) exprs);
- info->statOid = statOid;
- info->inherit = dataForm->stxdinherit;
- info->rel = rel;
- info->kind = STATS_EXT_DEPENDENCIES;
- info->keys = bms_copy(keys);
- info->exprs = exprs;
+ /*
+ * Modify the copies we obtain from the relcache to have the
+ * correct varno for the parent relation, so that they match
+ * up correctly against qual clauses.
+ */
+ if (varno != 1)
+ ChangeVarNodes((Node *) exprs, 1, varno, 0);
+ }
+ }
- stainfos = lappend(stainfos, info);
- }
+ /* add one StatisticExtInfo for each kind built */
+ if (statext_is_kind_built(dtup, STATS_EXT_NDISTINCT))
+ {
+ StatisticExtInfo *info = makeNode(StatisticExtInfo);
- if (statext_is_kind_built(dtup, STATS_EXT_MCV))
- {
- StatisticExtInfo *info = makeNode(StatisticExtInfo);
+ info->statOid = statOid;
+ info->inherit = dataForm->stxdinherit;
+ info->rel = rel;
+ info->kind = STATS_EXT_NDISTINCT;
+ info->keys = bms_copy(keys);
+ info->exprs = exprs;
- info->statOid = statOid;
- info->inherit = dataForm->stxdinherit;
- info->rel = rel;
- info->kind = STATS_EXT_MCV;
- info->keys = bms_copy(keys);
- info->exprs = exprs;
+ *stainfos = lappend(*stainfos, info);
+ }
- stainfos = lappend(stainfos, info);
- }
+ if (statext_is_kind_built(dtup, STATS_EXT_DEPENDENCIES))
+ {
+ StatisticExtInfo *info = makeNode(StatisticExtInfo);
- if (statext_is_kind_built(dtup, STATS_EXT_EXPRESSIONS))
- {
- StatisticExtInfo *info = makeNode(StatisticExtInfo);
+ info->statOid = statOid;
+ info->inherit = dataForm->stxdinherit;
+ info->rel = rel;
+ info->kind = STATS_EXT_DEPENDENCIES;
+ info->keys = bms_copy(keys);
+ info->exprs = exprs;
- info->statOid = statOid;
- info->inherit = dataForm->stxdinherit;
- info->rel = rel;
- info->kind = STATS_EXT_EXPRESSIONS;
- info->keys = bms_copy(keys);
- info->exprs = exprs;
+ *stainfos = lappend(*stainfos, info);
+ }
- stainfos = lappend(stainfos, info);
- }
+ if (statext_is_kind_built(dtup, STATS_EXT_MCV))
+ {
+ StatisticExtInfo *info = makeNode(StatisticExtInfo);
- ReleaseSysCache(dtup);
- }
+ info->statOid = statOid;
+ info->inherit = dataForm->stxdinherit;
+ info->rel = rel;
+ info->kind = STATS_EXT_MCV;
+ info->keys = bms_copy(keys);
+ info->exprs = exprs;
- ReleaseSysCache(htup);
- bms_free(keys);
+ *stainfos = lappend(*stainfos, info);
}
- list_free(statoidlist);
+ if (statext_is_kind_built(dtup, STATS_EXT_EXPRESSIONS))
+ {
+ StatisticExtInfo *info = makeNode(StatisticExtInfo);
- return stainfos;
+ info->statOid = statOid;
+ info->inherit = dataForm->stxdinherit;
+ info->rel = rel;
+ info->kind = STATS_EXT_EXPRESSIONS;
+ info->keys = bms_copy(keys);
+ info->exprs = exprs;
+
+ *stainfos = lappend(*stainfos, info);
+ }
+
+ ReleaseSysCache(dtup);
+ bms_free(keys);
}
/*
--
2.17.0
--qmeTShRBchLQhVIG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0006-Refactor-parent-ACL-check.patch"
view thread (22+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 05/15] Maybe better than looping twice. For partitioned tables, create only "inherited" stats_ext.
In-Reply-To: <no-message-id-1860763@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox