Some modest cleanups I've accumulated.
>From 9c5846cc3f04c6797696a234fac0953815e09e4b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 23 Oct 2022 14:52:48 -0500
Subject: [PATCH 1/6] WIP: do not loop to set an array to false
See also:
9fd45870c1436b477264c0c82eb195df52bc0919
572e3e6634e55accf95e2bcfb1340019c86a21dc
---
src/backend/access/transam/xlogprefetcher.c | 5 +---
src/backend/catalog/pg_constraint.c | 11 ++------
src/backend/commands/user.c | 6 +----
src/backend/statistics/extended_stats.c | 6 +----
src/backend/storage/smgr/md.c | 3 +--
src/backend/tcop/pquery.c | 3 +--
src/backend/utils/adt/arrayfuncs.c | 29 ++++++++-------------
src/test/isolation/isolationtester.c | 3 +--
8 files changed, 19 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/transam/xlogprefetcher.c b/src/backend/access/transam/xlogprefetcher.c
index 0cf03945eec..7fcfd146525 100644
--- a/src/backend/access/transam/xlogprefetcher.c
+++ b/src/backend/access/transam/xlogprefetcher.c
@@ -832,13 +832,10 @@ pg_stat_get_recovery_prefetch(PG_FUNCTION_ARGS)
#define PG_STAT_GET_RECOVERY_PREFETCH_COLS 10
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
Datum values[PG_STAT_GET_RECOVERY_PREFETCH_COLS];
- bool nulls[PG_STAT_GET_RECOVERY_PREFETCH_COLS];
+ bool nulls[PG_STAT_GET_RECOVERY_PREFETCH_COLS] = {0};
InitMaterializedSRF(fcinfo, 0);
- for (int i = 0; i < PG_STAT_GET_RECOVERY_PREFETCH_COLS; ++i)
- nulls[i] = false;
-
values[0] = TimestampTzGetDatum(pg_atomic_read_u64(&SharedStats->reset_time));
values[1] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->prefetch));
values[2] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->hit));
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3a5d4e96439..bd8f32bf62d 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -82,8 +82,8 @@ CreateConstraintEntry(const char *constraintName,
Relation conDesc;
Oid conOid;
HeapTuple tup;
- bool nulls[Natts_pg_constraint];
- Datum values[Natts_pg_constraint];
+ bool nulls[Natts_pg_constraint] = {0};
+ Datum values[Natts_pg_constraint] = {0};
ArrayType *conkeyArray;
ArrayType *confkeyArray;
ArrayType *conpfeqopArray;
@@ -165,13 +165,6 @@ CreateConstraintEntry(const char *constraintName,
else
conexclopArray = NULL;
- /* initialize nulls and values */
- for (i = 0; i < Natts_pg_constraint; i++)
- {
- nulls[i] = false;
- values[i] = (Datum) NULL;
- }
-
conOid = GetNewOidWithIndex(conDesc, ConstraintOidIndexId,
Anum_pg_constraint_oid);
values[Anum_pg_constraint_oid - 1] = ObjectIdGetDatum(conOid);
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8b6543edee2..5af79792136 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1234,8 +1234,7 @@ RenameRole(const char *oldname, const char *newname)
bool isnull;
Datum repl_val[Natts_pg_authid];
bool repl_null[Natts_pg_authid];
- bool repl_repl[Natts_pg_authid];
- int i;
+ bool repl_repl[Natts_pg_authid] = {0};
Oid roleid;
ObjectAddress address;
Form_pg_authid authform;
@@ -1321,9 +1320,6 @@ RenameRole(const char *oldname, const char *newname)
}
/* OK, construct the modified tuple */
- for (i = 0; i < Natts_pg_authid; i++)
- repl_repl[i] = false;
-
repl_repl[Anum_pg_authid_rolname - 1] = true;
repl_val[Anum_pg_authid_rolname - 1] = DirectFunctionCall1(namein,
CStringGetDatum(newname));
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index ab97e71dd79..93a9d3eeafb 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -2343,7 +2343,7 @@ serialize_expr_stats(AnlExprData *exprdata, int nexprs)
VacAttrStats *stats = exprdata[exprno].vacattrstat;
Datum values[Natts_pg_statistic];
- bool nulls[Natts_pg_statistic];
+ bool nulls[Natts_pg_statistic] = {0};
HeapTuple stup;
if (!stats->stats_valid)
@@ -2359,10 +2359,6 @@ serialize_expr_stats(AnlExprData *exprdata, int nexprs)
/*
* Construct a new pg_statistic tuple
*/
- for (i = 0; i < Natts_pg_statistic; ++i)
- {
- nulls[i] = false;
- }
values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(InvalidOid);
values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(InvalidAttrNumber);
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 14b6fa0fd90..2d5cc97eaa9 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -557,8 +557,7 @@ void
mdopen(SMgrRelation reln)
{
/* mark it not open */
- for (int forknum = 0; forknum <= MAX_FORKNUM; forknum++)
- reln->md_num_open_segs[forknum] = 0;
+ memset(reln->md_num_open_segs, 0, (1+MAX_FORKNUM) * sizeof(*reln->md_num_open_segs));
}
/*
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 52e2db6452b..53b712fd3b5 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -653,8 +653,7 @@ PortalSetResultFormat(Portal portal, int nFormats, int16 *formats)
else
{
/* use default format for all columns */
- for (i = 0; i < natts; i++)
- portal->formats[i] = 0;
+ memset(portal->formats, 0, natts * sizeof(*portal->formats));
}
}
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 495e449a9e9..ff62eb01dfd 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1038,7 +1038,7 @@ array_out(PG_FUNCTION_ARGS)
i,
j,
k,
- indx[MAXDIM];
+ indx[MAXDIM] = {0};
int ndim,
*dims,
*lb;
@@ -1203,8 +1203,7 @@ array_out(PG_FUNCTION_ARGS)
if (needdims)
APPENDSTR(dims_str);
APPENDCHAR('{');
- for (i = 0; i < ndim; i++)
- indx[i] = 0;
+
j = 0;
k = 0;
do
@@ -4988,10 +4987,9 @@ array_slice_size(char *arraydataptr, bits8 *arraynullsptr,
span[MAXDIM],
prod[MAXDIM],
dist[MAXDIM],
- indx[MAXDIM];
+ indx[MAXDIM] = {0};
char *ptr;
- int i,
- j,
+ int j,
inc;
int count = 0;
@@ -5007,8 +5005,7 @@ array_slice_size(char *arraydataptr, bits8 *arraynullsptr,
typlen, typbyval, typalign);
mda_get_prod(ndim, dim, prod);
mda_get_offset_values(ndim, dist, prod, span);
- for (i = 0; i < ndim; i++)
- indx[i] = 0;
+
j = ndim - 1;
do
{
@@ -5059,9 +5056,8 @@ array_extract_slice(ArrayType *newarray,
prod[MAXDIM],
span[MAXDIM],
dist[MAXDIM],
- indx[MAXDIM];
- int i,
- j,
+ indx[MAXDIM] = {0};
+ int j,
inc;
src_offset = ArrayGetOffset(ndim, dim, lb, st);
@@ -5070,8 +5066,7 @@ array_extract_slice(ArrayType *newarray,
mda_get_prod(ndim, dim, prod);
mda_get_range(ndim, span, st, endp);
mda_get_offset_values(ndim, dist, prod, span);
- for (i = 0; i < ndim; i++)
- indx[i] = 0;
+
dest_offset = 0;
j = ndim - 1;
do
@@ -5138,9 +5133,8 @@ array_insert_slice(ArrayType *destArray,
prod[MAXDIM],
span[MAXDIM],
dist[MAXDIM],
- indx[MAXDIM];
- int i,
- j,
+ indx[MAXDIM] = {0};
+ int j,
inc;
dest_offset = ArrayGetOffset(ndim, dim, lb, st);
@@ -5156,8 +5150,7 @@ array_insert_slice(ArrayType *destArray,
mda_get_prod(ndim, dim, prod);
mda_get_range(ndim, span, st, endp);
mda_get_offset_values(ndim, dist, prod, span);
- for (i = 0; i < ndim; i++)
- indx[i] = 0;
+
src_offset = 0;
j = ndim - 1;
do
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 0a66235153a..19808c59892 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -432,8 +432,7 @@ run_all_permutations(TestSpec *testspec)
* already picked from this pile.
*/
piles = pg_malloc(sizeof(int) * testspec->nsessions);
- for (i = 0; i < testspec->nsessions; i++)
- piles[i] = 0;
+ memset(piles, 0, sizeof(*piles) * testspec->nsessions);
run_all_permutations_recurse(testspec, piles, 0, stepptrs);
--
2.25.1
>From cc9f9b8b878b4d878b0aa4f04226cb6c2873275e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 14 Jan 2022 19:25:21 -0600
Subject: [PATCH 2/6] Avoid the most useless instances of nulls[0]=false..
Getting rid of more of these is apparently too controvercial, but these are the
cases where 1) nulls is always being set to false; and, 2) values[] is not
being memset, so there's no parallel between the two.
---
contrib/sslinfo/sslinfo.c | 5 +-
src/backend/replication/logical/origin.c | 13 +---
src/backend/utils/adt/misc.c | 7 +--
src/backend/utils/misc/pg_controldata.c | 80 ++----------------------
4 files changed, 10 insertions(+), 95 deletions(-)
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index 5fd46b98741..d9edfe1b742 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -424,7 +424,7 @@ ssl_extension_info(PG_FUNCTION_ARGS)
if (call_cntr < max_calls)
{
Datum values[3];
- bool nulls[3];
+ bool nulls[3] = {0};
char *buf;
HeapTuple tuple;
Datum result;
@@ -453,7 +453,6 @@ ssl_extension_info(PG_FUNCTION_ARGS)
errmsg("unknown OpenSSL extension in certificate at position %d",
call_cntr)));
values[0] = CStringGetTextDatum(OBJ_nid2sn(nid));
- nulls[0] = false;
/* Get the extension value */
if (X509V3_EXT_print(membuf, ext, 0, 0) <= 0)
@@ -463,11 +462,9 @@ ssl_extension_info(PG_FUNCTION_ARGS)
call_cntr)));
len = BIO_get_mem_data(membuf, &buf);
values[1] = PointerGetDatum(cstring_to_text_with_len(buf, len));
- nulls[1] = false;
/* Get critical status */
values[2] = BoolGetDatum(X509_EXTENSION_get_critical(ext));
- nulls[2] = false;
/* Build tuple */
tuple = heap_form_tuple(fctx->tupdesc, values, nulls);
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index f134e44878f..adb3430c33a 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -1527,10 +1527,9 @@ pg_show_replication_origin_status(PG_FUNCTION_ARGS)
continue;
memset(values, 0, sizeof(values));
- memset(nulls, 1, sizeof(nulls));
+ memset(nulls, 0, sizeof(nulls));
values[0] = ObjectIdGetDatum(state->roident);
- nulls[0] = false;
/*
* We're not preventing the origin to be dropped concurrently, so
@@ -1538,19 +1537,13 @@ pg_show_replication_origin_status(PG_FUNCTION_ARGS)
*/
if (replorigin_by_oid(state->roident, true,
&roname))
- {
values[1] = CStringGetTextDatum(roname);
- nulls[1] = false;
- }
+ else
+ nulls[1] = true;
LWLockAcquire(&state->lock, LW_SHARED);
-
values[2] = LSNGetDatum(state->remote_lsn);
- nulls[2] = false;
-
values[3] = LSNGetDatum(state->local_lsn);
- nulls[3] = false;
-
LWLockRelease(&state->lock);
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 9c132512315..611b492624a 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -246,7 +246,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
char *subdir;
bool isempty;
Datum values[1];
- bool nulls[1];
+ bool nulls[1] = {0};
/* this test skips . and .., but is awfully weak */
if (!datOid)
@@ -262,7 +262,6 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
continue; /* indeed, nothing in it */
values[0] = ObjectIdGetDatum(datOid);
- nulls[0] = false;
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
values, nulls);
@@ -530,11 +529,9 @@ pg_get_catalog_foreign_keys(PG_FUNCTION_ARGS)
{
const SysFKRelationship *fkrel = &sys_fk_relationships[funcctx->call_cntr];
Datum values[6];
- bool nulls[6];
+ bool nulls[6] = {0};
HeapTuple tuple;
- memset(nulls, false, sizeof(nulls));
-
values[0] = ObjectIdGetDatum(fkrel->fk_table);
values[1] = FunctionCall3(arrayinp,
CStringGetDatum(fkrel->fk_columns),
diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 781f8b87580..32be2b4b5d8 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -32,7 +32,7 @@ Datum
pg_control_system(PG_FUNCTION_ARGS)
{
Datum values[4];
- bool nulls[4];
+ bool nulls[4] = {0};
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
@@ -60,16 +60,9 @@ pg_control_system(PG_FUNCTION_ARGS)
(errmsg("calculated CRC checksum does not match value stored in file")));
values[0] = Int32GetDatum(ControlFile->pg_control_version);
- nulls[0] = false;
-
values[1] = Int32GetDatum(ControlFile->catalog_version_no);
- nulls[1] = false;
-
values[2] = Int64GetDatum(ControlFile->system_identifier);
- nulls[2] = false;
-
values[3] = TimestampTzGetDatum(time_t_to_timestamptz(ControlFile->time));
- nulls[3] = false;
htup = heap_form_tuple(tupdesc, values, nulls);
@@ -80,7 +73,7 @@ Datum
pg_control_checkpoint(PG_FUNCTION_ARGS)
{
Datum values[18];
- bool nulls[18];
+ bool nulls[18] = {0};
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
@@ -147,60 +140,25 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
/* Populate the values and null arrays */
values[0] = LSNGetDatum(ControlFile->checkPoint);
- nulls[0] = false;
-
values[1] = LSNGetDatum(ControlFile->checkPointCopy.redo);
- nulls[1] = false;
-
values[2] = CStringGetTextDatum(xlogfilename);
- nulls[2] = false;
-
values[3] = Int32GetDatum(ControlFile->checkPointCopy.ThisTimeLineID);
- nulls[3] = false;
-
values[4] = Int32GetDatum(ControlFile->checkPointCopy.PrevTimeLineID);
- nulls[4] = false;
-
values[5] = BoolGetDatum(ControlFile->checkPointCopy.fullPageWrites);
- nulls[5] = false;
-
values[6] = CStringGetTextDatum(psprintf("%u:%u",
EpochFromFullTransactionId(ControlFile->checkPointCopy.nextXid),
XidFromFullTransactionId(ControlFile->checkPointCopy.nextXid)));
- nulls[6] = false;
-
values[7] = ObjectIdGetDatum(ControlFile->checkPointCopy.nextOid);
- nulls[7] = false;
-
values[8] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMulti);
- nulls[8] = false;
-
values[9] = TransactionIdGetDatum(ControlFile->checkPointCopy.nextMultiOffset);
- nulls[9] = false;
-
values[10] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestXid);
- nulls[10] = false;
-
values[11] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestXidDB);
- nulls[11] = false;
-
values[12] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestActiveXid);
- nulls[12] = false;
-
values[13] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestMulti);
- nulls[13] = false;
-
values[14] = ObjectIdGetDatum(ControlFile->checkPointCopy.oldestMultiDB);
- nulls[14] = false;
-
values[15] = TransactionIdGetDatum(ControlFile->checkPointCopy.oldestCommitTsXid);
- nulls[15] = false;
-
values[16] = TransactionIdGetDatum(ControlFile->checkPointCopy.newestCommitTsXid);
- nulls[16] = false;
-
values[17] = TimestampTzGetDatum(time_t_to_timestamptz(ControlFile->checkPointCopy.time));
- nulls[17] = false;
htup = heap_form_tuple(tupdesc, values, nulls);
@@ -211,7 +169,7 @@ Datum
pg_control_recovery(PG_FUNCTION_ARGS)
{
Datum values[5];
- bool nulls[5];
+ bool nulls[5] = {0};
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
@@ -241,19 +199,10 @@ pg_control_recovery(PG_FUNCTION_ARGS)
(errmsg("calculated CRC checksum does not match value stored in file")));
values[0] = LSNGetDatum(ControlFile->minRecoveryPoint);
- nulls[0] = false;
-
values[1] = Int32GetDatum(ControlFile->minRecoveryPointTLI);
- nulls[1] = false;
-
values[2] = LSNGetDatum(ControlFile->backupStartPoint);
- nulls[2] = false;
-
values[3] = LSNGetDatum(ControlFile->backupEndPoint);
- nulls[3] = false;
-
values[4] = BoolGetDatum(ControlFile->backupEndRequired);
- nulls[4] = false;
htup = heap_form_tuple(tupdesc, values, nulls);
@@ -264,7 +213,7 @@ Datum
pg_control_init(PG_FUNCTION_ARGS)
{
Datum values[11];
- bool nulls[11];
+ bool nulls[11] = {0};
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
@@ -306,37 +255,16 @@ pg_control_init(PG_FUNCTION_ARGS)
(errmsg("calculated CRC checksum does not match value stored in file")));
values[0] = Int32GetDatum(ControlFile->maxAlign);
- nulls[0] = false;
-
values[1] = Int32GetDatum(ControlFile->blcksz);
- nulls[1] = false;
-
values[2] = Int32GetDatum(ControlFile->relseg_size);
- nulls[2] = false;
-
values[3] = Int32GetDatum(ControlFile->xlog_blcksz);
- nulls[3] = false;
-
values[4] = Int32GetDatum(ControlFile->xlog_seg_size);
- nulls[4] = false;
-
values[5] = Int32GetDatum(ControlFile->nameDataLen);
- nulls[5] = false;
-
values[6] = Int32GetDatum(ControlFile->indexMaxKeys);
- nulls[6] = false;
-
values[7] = Int32GetDatum(ControlFile->toast_max_chunk_size);
- nulls[7] = false;
-
values[8] = Int32GetDatum(ControlFile->loblksize);
- nulls[8] = false;
-
values[9] = BoolGetDatum(ControlFile->float8ByVal);
- nulls[9] = false;
-
values[10] = Int32GetDatum(ControlFile->data_checksum_version);
- nulls[10] = false;
htup = heap_form_tuple(tupdesc, values, nulls);
--
2.25.1
>From 54dfbc7d0081de75546003b4c401785127921168 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 25 Sep 2021 18:58:33 -0500
Subject: [PATCH 3/6] selfuncs.c: refactor parent ACL check
The code for statistics expressions was copied from indexes, and this
un-copies it. selfuncs.c is 8k lines long, and this makes it 30 LOC shorter.
See prior discussion:
f97a5902-b4d1-77da-8d2f-6b5b2094f...@enterprisedb.com
Tomas said there's similar code elsewhere, and I saw
examine_simple_variable() and ruleutils.c, but I'm unable to find
anything else similar close enough to share.
---
src/backend/utils/adt/selfuncs.c | 140 ++++++++++++-------------------
1 file changed, 52 insertions(+), 88 deletions(-)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index e0aeaa69092..a9f3ccf9a30 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -188,6 +188,8 @@ static char *convert_string_datum(Datum value, Oid typid, Oid collid,
bool *failure);
static double convert_timevalue_to_scalar(Datum value, Oid typid,
bool *failure);
+static void recheck_parent_acl(PlannerInfo *root, VariableStatData *vardata,
+ Oid relid);
static void examine_simple_variable(PlannerInfo *root, Var *var,
VariableStatData *vardata);
static bool get_variable_range(PlannerInfo *root, VariableStatData *vardata,
@@ -5174,51 +5176,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
(pg_class_aclcheck(rte->relid, userid,
ACL_SELECT) == ACLCHECK_OK);
- /*
- * If the user doesn't have permissions to
- * access an inheritance child relation, check
- * the permissions of the table actually
- * mentioned in the query, since most likely
- * the user does have that permission. Note
- * that whole-table select privilege on the
- * parent doesn't quite guarantee that the
- * user could read all columns of the child.
- * But in practice it's unlikely that any
- * interesting security violation could result
- * from allowing access to the expression
- * index's stats, so we allow it anyway. See
- * similar code in examine_simple_variable()
- * for additional comments.
- */
- if (!vardata->acl_ok &&
- root->append_rel_array != NULL)
- {
- AppendRelInfo *appinfo;
- Index varno = index->rel->relid;
-
- appinfo = root->append_rel_array[varno];
- while (appinfo &&
- planner_rt_fetch(appinfo->parent_relid,
- root)->rtekind == RTE_RELATION)
- {
- varno = appinfo->parent_relid;
- appinfo = root->append_rel_array[varno];
- }
- if (varno != index->rel->relid)
- {
- /* Repeat access check on this rel */
- rte = planner_rt_fetch(varno, root);
- Assert(rte->rtekind == RTE_RELATION);
-
- userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
- vardata->acl_ok =
- rte->securityQuals == NIL &&
- (pg_class_aclcheck(rte->relid,
- userid,
- ACL_SELECT) == ACLCHECK_OK);
- }
- }
+ recheck_parent_acl(root, vardata, index->rel->relid);
}
else
{
@@ -5308,49 +5266,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
(pg_class_aclcheck(rte->relid, userid,
ACL_SELECT) == ACLCHECK_OK);
- /*
- * If the user doesn't have permissions to access an
- * inheritance child relation, check the permissions of
- * the table actually mentioned in the query, since most
- * likely the user does have that permission. Note that
- * whole-table select privilege on the parent doesn't
- * quite guarantee that the user could read all columns of
- * the child. But in practice it's unlikely that any
- * interesting security violation could result from
- * allowing access to the expression stats, so we allow it
- * anyway. See similar code in examine_simple_variable()
- * for additional comments.
- */
- if (!vardata->acl_ok &&
- root->append_rel_array != NULL)
- {
- AppendRelInfo *appinfo;
- Index varno = onerel->relid;
-
- appinfo = root->append_rel_array[varno];
- while (appinfo &&
- planner_rt_fetch(appinfo->parent_relid,
- root)->rtekind == RTE_RELATION)
- {
- varno = appinfo->parent_relid;
- appinfo = root->append_rel_array[varno];
- }
- if (varno != onerel->relid)
- {
- /* Repeat access check on this rel */
- rte = planner_rt_fetch(varno, root);
- Assert(rte->rtekind == RTE_RELATION);
-
- userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
- vardata->acl_ok =
- rte->securityQuals == NIL &&
- (pg_class_aclcheck(rte->relid,
- userid,
- ACL_SELECT) == ACLCHECK_OK);
- }
- }
-
+ recheck_parent_acl(root, vardata, onerel->relid);
break;
}
@@ -5360,6 +5276,54 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
}
}
+/*
+ * If the user doesn't have permissions to access an inheritance child
+ * relation, check the permissions of the table actually mentioned in the
+ * query, since most likely the user does have that permission. Note that
+ * whole-table select privilege on the parent doesn't quite guarantee that the
+ * user could read all columns of the child. But in practice it's unlikely
+ * that any interesting security violation could result from allowing access to
+ * the index or expression stats, so we allow it anyway. See similar code in
+ * examine_simple_variable() for additional comments.
+ */
+static void
+recheck_parent_acl(PlannerInfo *root, VariableStatData *vardata, Oid relid)
+{
+ RangeTblEntry *rte;
+ Oid userid;
+
+ if (!vardata->acl_ok &&
+ root->append_rel_array != NULL)
+ {
+ AppendRelInfo *appinfo;
+ Index varno = relid;
+
+ appinfo = root->append_rel_array[varno];
+ while (appinfo &&
+ planner_rt_fetch(appinfo->parent_relid,
+ root)->rtekind == RTE_RELATION)
+ {
+ varno = appinfo->parent_relid;
+ appinfo = root->append_rel_array[varno];
+ }
+
+ if (varno != relid)
+ {
+ /* Repeat access check on this rel */
+ rte = planner_rt_fetch(varno, root);
+ Assert(rte->rtekind == RTE_RELATION);
+
+ userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+
+ vardata->acl_ok =
+ rte->securityQuals == NIL &&
+ (pg_class_aclcheck(rte->relid,
+ userid,
+ ACL_SELECT) == ACLCHECK_OK);
+ }
+ }
+}
+
/*
* examine_simple_variable
* Handle a simple Var for examine_variable
--
2.25.1
>From 28ffc13ceab70b9e05aa3563b0b8b37e209a349e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 25 Nov 2021 22:47:41 -0600
Subject: [PATCH 4/6] move beentry++ away from the middle of the loop body
This was more reasonable when the "if" block was shorter; but, now it's
large, and the variable increment is easy to miss, as happened during
patch development:
https://www.postgresql.org/message-id/20211202040653.gm17...@telsasoft.com
---
src/backend/utils/activity/backend_status.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 1146a6c33cd..1cf2bc58410 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -842,7 +842,6 @@ pgstat_read_current_status(void)
CHECK_FOR_INTERRUPTS();
}
- beentry++;
/* Only valid entries get included into the local array */
if (localentry->backendStatus.st_procpid > 0)
{
@@ -869,6 +868,8 @@ pgstat_read_current_status(void)
#endif
localNumBackends++;
}
+
+ beentry++;
}
/* Set the pointer only after completion of a valid table */
--
2.25.1
>From 51e25dc00d811965c6e72d1914bd03035d9797fe Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 22 Nov 2022 18:44:23 -0600
Subject: [PATCH 5/6] conjunction is cleaner and shorter than nested
conditionals
---
src/backend/replication/pgoutput/pgoutput.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 2ecaa5b9074..2ec201e3007 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -810,7 +810,7 @@ pgoutput_row_filter_exec_expr(ExprState *state, ExprContext *econtext)
ret = ExecEvalExprSwitchContext(state, econtext, &isnull);
elog(DEBUG3, "row filter evaluates to %s (isnull: %s)",
- isnull ? "false" : DatumGetBool(ret) ? "true" : "false",
+ !isnull && DatumGetBool(ret) ? "true" : "false",
isnull ? "true" : "false");
if (isnull)
--
2.25.1
>From f3c4c2cf735c448edd66e9f0f546547355aff1c1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 13 Jul 2022 15:06:21 -0500
Subject: [PATCH 6/6] basebackup: use port/strlcpy()
---
src/bin/pg_basebackup/pg_basebackup.c | 3 +--
src/bin/pg_basebackup/pg_receivewal.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 22836ca01a1..28960158e7f 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1005,8 +1005,7 @@ parse_compress_options(char *option, char **algorithm, char **detail,
char *alg;
alg = palloc((sep - option) + 1);
- memcpy(alg, option, sep - option);
- alg[sep - option] = '\0';
+ strlcpy(alg, option, sep - option + 1);
*algorithm = alg;
*detail = pstrdup(sep + 1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 63207ca025e..3acf19a1201 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -161,8 +161,7 @@ parse_compress_options(char *option, char **algorithm, char **detail)
char *alg;
alg = palloc((sep - option) + 1);
- memcpy(alg, option, sep - option);
- alg[sep - option] = '\0';
+ strlcpy(alg, option, sep - option + 1);
*algorithm = alg;
*detail = pstrdup(sep + 1);
--
2.25.1
