properly sizing attnums in pg_get_publication_tables
Hi,
I was looking at commit b7ae03953690a1dee455ba3823cc8f71a72cbe1d .
In `pg_get_publication_tables`, attnums is allocated with size
`desc->natts`. However, since some columns may be dropped, this size may be
larger than necessary.
When `nattnums > 0` is false, there is no need to allocate the `attnums`
array. In the current formation, `attnums` should be freed in this scenario.
Please take a look at the patch which moves the allocation to inside the
`if (nattnums > 0)` block.
Thanks
Attachments:
proper-sizing-of-attnums.patchapplication/octet-stream; name=proper-sizing-of-attnums.patchDownload
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index a98fcad421..1e953b1613 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -1160,9 +1160,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
int nattnums = 0;
int16 *attnums;
TupleDesc desc = RelationGetDescr(rel);
- int i;
-
- attnums = (int16 *) palloc(desc->natts * sizeof(int16));
+ int i, j;
for (i = 0; i < desc->natts; i++)
{
@@ -1171,11 +1169,23 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
if (att->attisdropped || att->attgenerated)
continue;
- attnums[nattnums++] = att->attnum;
+ nattnums++;
}
if (nattnums > 0)
{
+ attnums = (int16 *) palloc(nattnums * sizeof(int16));
+
+ for (i = 0, j = 0; i < desc->natts; i++)
+ {
+ Form_pg_attribute att = TupleDescAttr(desc, i);
+
+ if (att->attisdropped || att->attgenerated)
+ continue;
+
+ attnums[j++] = att->attnum;
+ }
+
values[1] = PointerGetDatum(buildint2vector(attnums, nattnums));
nulls[1] = false;
}
On Fri, Jan 13, 2023 at 07:37:29AM -0800, Ted Yu wrote:
Hi,
I was looking at commit b7ae03953690a1dee455ba3823cc8f71a72cbe1d .In `pg_get_publication_tables`, attnums is allocated with size
`desc->natts`. However, since some columns may be dropped, this size may be
larger than necessary.
When `nattnums > 0` is false, there is no need to allocate the `attnums`
array. In the current formation, `attnums` should be freed in this scenario.Please take a look at the patch which moves the allocation to inside the
`if (nattnums > 0)` block.Thanks
It doesn't seem worth the bother of changing it or adding 10 lines of
code, or keeping track of whether "attnums" is initialized or not.
After all, it wasn't worth pfree()ing the array (which seems to work as
intended). The array can't be larger than ~3200 bytes, and I doubt
anybody is going to be excited about saving a couple bytes per dropped
column.
--
Justin