Use get_call_result_type() more widely
Hi,
A review comment in another thread [1]/messages/by-id/Y41De5NnF2sxmJPI@paquier.xyz by Michael Paquier about the
usage of get_call_result_type() instead of explicit building of
TupleDesc made me think about using it more widely. Actually, the
get_call_result_type() looks at the function definitions to figure the
column names and build the required TupleDesc, usage of which avoids
duplication of the column names between pg_proc.dat/function
definitions and source code. Also, it saves a good number of LOC ~415
[2]: 21 files changed, 97 insertions(+), 514 deletions(-)
~4MB, which means, the postgres binary becomes leaner by ~4MB [3]Source code is built with CFLAGS = -O3. PATCHED: text data bss dec hex filename 1043 0 0 1043 413 contrib/old_snapshot/time_mapping.o 7192 0 0 7192 1c18 contrib/pg_visibility/pg_visibility.o 7144 0 120 7264 1c60 src/backend/access/transam/commit_ts.o 19681 24 248 19953 4df1 src/backend/access/transam/multixact.o 20595 0 88 20683 50cb src/backend/access/transam/twophase.o 6162 0 24 6186 182a src/backend/access/transam/xlogfuncs.o 45540 2736 8 48284 bc9c src/backend/catalog/objectaddress.o 9943 0 0 9943 26d7 src/backend/catalog/pg_publication.o 18239 0 16 18255 474f src/backend/commands/sequence.o 6429 0 0 6429 191d src/backend/tsearch/wparser.o 47049 1840 52 48941 bf2d src/backend/utils/adt/acl.o 43066 168 784 44018 abf2 src/backend/utils/adt/datetime.o 6843 0 0 6843 1abb src/backend/utils/adt/genfile.o 6904 120 0 7024 1b70 src/backend/utils/adt/lockfuncs.o 10512 7008 0 17520 4470 src/backend/utils/adt/misc.o 1569 0 0 1569 621 src/backend/utils/adt/partitionfuncs.o 16266 0 0 16266 3f8a src/backend/utils/adt/pgstatfuncs.o 40985 0 0 40985 a019 src/backend/utils/adt/tsvector_op.o 8322 0 0 8322 2082 src/backend/utils/misc/guc_funcs.o 2109 0 0 2109 83d src/backend/utils/misc/pg_controldata.o 2354 0 0 2354 932 src/test/modules/test_predtest/test_predtest.o 9586047 226936 205536 10018519 98ded7 src/backend/postgres. I'm
attaching a patch for these changes.
While on this, I observed that BlessTupleDesc() is called in many
(~12) places right after get_call_result_type() which actually does
the job of BlessTupleDesc() before returning the TupleDesc. I think we
can get rid of BlessTupleDesc() after get_call_result_type(). I'm
attaching a patch for these changes too.
cirrus-ci members are happy with these patches, please see here
https://github.com/BRupireddy/postgres/tree/use_get_call_result_type()_more_widely_v1.
Thoughts?
[1]: /messages/by-id/Y41De5NnF2sxmJPI@paquier.xyz
[2]: 21 files changed, 97 insertions(+), 514 deletions(-)
[3]: Source code is built with CFLAGS = -O3. PATCHED: text data bss dec hex filename 1043 0 0 1043 413 contrib/old_snapshot/time_mapping.o 7192 0 0 7192 1c18 contrib/pg_visibility/pg_visibility.o 7144 0 120 7264 1c60 src/backend/access/transam/commit_ts.o 19681 24 248 19953 4df1 src/backend/access/transam/multixact.o 20595 0 88 20683 50cb src/backend/access/transam/twophase.o 6162 0 24 6186 182a src/backend/access/transam/xlogfuncs.o 45540 2736 8 48284 bc9c src/backend/catalog/objectaddress.o 9943 0 0 9943 26d7 src/backend/catalog/pg_publication.o 18239 0 16 18255 474f src/backend/commands/sequence.o 6429 0 0 6429 191d src/backend/tsearch/wparser.o 47049 1840 52 48941 bf2d src/backend/utils/adt/acl.o 43066 168 784 44018 abf2 src/backend/utils/adt/datetime.o 6843 0 0 6843 1abb src/backend/utils/adt/genfile.o 6904 120 0 7024 1b70 src/backend/utils/adt/lockfuncs.o 10512 7008 0 17520 4470 src/backend/utils/adt/misc.o 1569 0 0 1569 621 src/backend/utils/adt/partitionfuncs.o 16266 0 0 16266 3f8a src/backend/utils/adt/pgstatfuncs.o 40985 0 0 40985 a019 src/backend/utils/adt/tsvector_op.o 8322 0 0 8322 2082 src/backend/utils/misc/guc_funcs.o 2109 0 0 2109 83d src/backend/utils/misc/pg_controldata.o 2354 0 0 2354 932 src/test/modules/test_predtest/test_predtest.o 9586047 226936 205536 10018519 98ded7 src/backend/postgres
PATCHED:
text data bss dec hex filename
1043 0 0 1043 413 contrib/old_snapshot/time_mapping.o
7192 0 0 7192 1c18 contrib/pg_visibility/pg_visibility.o
7144 0 120 7264 1c60 src/backend/access/transam/commit_ts.o
19681 24 248 19953 4df1 src/backend/access/transam/multixact.o
20595 0 88 20683 50cb src/backend/access/transam/twophase.o
6162 0 24 6186 182a src/backend/access/transam/xlogfuncs.o
45540 2736 8 48284 bc9c src/backend/catalog/objectaddress.o
9943 0 0 9943 26d7 src/backend/catalog/pg_publication.o
18239 0 16 18255 474f src/backend/commands/sequence.o
6429 0 0 6429 191d src/backend/tsearch/wparser.o
47049 1840 52 48941 bf2d src/backend/utils/adt/acl.o
43066 168 784 44018 abf2 src/backend/utils/adt/datetime.o
6843 0 0 6843 1abb src/backend/utils/adt/genfile.o
6904 120 0 7024 1b70 src/backend/utils/adt/lockfuncs.o
10512 7008 0 17520 4470 src/backend/utils/adt/misc.o
1569 0 0 1569 621 src/backend/utils/adt/partitionfuncs.o
16266 0 0 16266 3f8a src/backend/utils/adt/pgstatfuncs.o
40985 0 0 40985 a019 src/backend/utils/adt/tsvector_op.o
8322 0 0 8322 2082 src/backend/utils/misc/guc_funcs.o
2109 0 0 2109 83d src/backend/utils/misc/pg_controldata.o
2354 0 0 2354 932
src/test/modules/test_predtest/test_predtest.o
9586047 226936 205536 10018519 98ded7 src/backend/postgres
HEAD:
text data bss dec hex filename
1019 0 0 1019 3fb contrib/old_snapshot/time_mapping.o
7159 0 0 7159 1bf7 contrib/pg_visibility/pg_visibility.o
6655 0 120 6775 1a77 src/backend/access/transam/commit_ts.o
19636 24 248 19908 4dc4 src/backend/access/transam/multixact.o
20663 0 88 20751 510f src/backend/access/transam/twophase.o
6206 0 24 6230 1856 src/backend/access/transam/xlogfuncs.o
45700 2736 8 48444 bd3c src/backend/catalog/objectaddress.o
9952 0 0 9952 26e0 src/backend/catalog/pg_publication.o
18487 0 16 18503 4847 src/backend/commands/sequence.o
6143 0 0 6143 17ff src/backend/tsearch/wparser.o
47123 1840 52 49015 bf77 src/backend/utils/adt/acl.o
43099 168 784 44051 ac13 src/backend/utils/adt/datetime.o
7016 0 0 7016 1b68 src/backend/utils/adt/genfile.o
7413 120 0 7533 1d6d src/backend/utils/adt/lockfuncs.o
10698 7008 0 17706 452a src/backend/utils/adt/misc.o
1593 0 0 1593 639 src/backend/utils/adt/partitionfuncs.o
17194 0 0 17194 432a src/backend/utils/adt/pgstatfuncs.o
40798 0 0 40798 9f5e src/backend/utils/adt/tsvector_op.o
8871 0 0 8871 22a7 src/backend/utils/misc/guc_funcs.o
3918 0 0 3918 f4e src/backend/utils/misc/pg_controldata.o
2636 0 0 2636 a4c
src/test/modules/test_predtest/test_predtest.o
9589943 226936 205536 10022415 98ee0f src/backend/postgres
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Use-get_call_result_type-more-widely.patchapplication/x-patch; name=v1-0001-Use-get_call_result_type-more-widely.patchDownload
From e1df78cc86e3d38a2814d6cd89f6b86a8de4a284 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 13 Dec 2022 07:04:22 +0000
Subject: [PATCH v1] Use get_call_result_type() more widely
Usage of get_call_result_type() in more places avoids explicit
creation of tuple descriptor which requires matching column names
from pg_proc.dat/function definitions. It saves a good amount of
LOC ~415 and the size of all the object files put together gets
reduced by ~4MB, which means, the postgres binary can become
leaner by ~4MB.
---
contrib/old_snapshot/time_mapping.c | 26 +----
contrib/pg_visibility/pg_visibility.c | 6 +-
src/backend/access/transam/commit_ts.c | 26 +----
src/backend/access/transam/multixact.c | 9 +-
src/backend/access/transam/twophase.c | 17 +--
src/backend/access/transam/xlogfuncs.c | 21 +---
src/backend/catalog/objectaddress.c | 42 +------
src/backend/catalog/pg_publication.c | 13 +--
src/backend/commands/sequence.c | 19 +--
src/backend/tsearch/wparser.c | 29 +++--
src/backend/utils/adt/acl.c | 18 +--
src/backend/utils/adt/datetime.c | 17 +--
src/backend/utils/adt/genfile.c | 20 +---
src/backend/utils/adt/lockfuncs.c | 40 +------
src/backend/utils/adt/misc.c | 31 +----
src/backend/utils/adt/partitionfuncs.c | 14 +--
src/backend/utils/adt/pgstatfuncs.c | 78 ++-----------
src/backend/utils/adt/tsvector_op.c | 15 +--
src/backend/utils/misc/guc_funcs.c | 42 +------
src/backend/utils/misc/pg_controldata.c | 108 ++----------------
.../modules/test_predtest/test_predtest.c | 20 +---
21 files changed, 97 insertions(+), 514 deletions(-)
diff --git a/contrib/old_snapshot/time_mapping.c b/contrib/old_snapshot/time_mapping.c
index 2d8cb742c3..97efccddbb 100644
--- a/contrib/old_snapshot/time_mapping.c
+++ b/contrib/old_snapshot/time_mapping.c
@@ -38,7 +38,6 @@ PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(pg_old_snapshot_time_mapping);
static OldSnapshotTimeMapping *GetOldSnapshotTimeMapping(void);
-static TupleDesc MakeOldSnapshotTimeMappingTupleDesc(void);
static HeapTuple MakeOldSnapshotTimeMappingTuple(TupleDesc tupdesc,
OldSnapshotTimeMapping *mapping);
@@ -54,12 +53,15 @@ pg_old_snapshot_time_mapping(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
MemoryContext oldcontext;
+ TupleDesc tupdesc;
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
mapping = GetOldSnapshotTimeMapping();
funcctx->user_fctx = mapping;
- funcctx->tuple_desc = MakeOldSnapshotTimeMappingTupleDesc();
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
MemoryContextSwitchTo(oldcontext);
}
@@ -101,26 +103,6 @@ GetOldSnapshotTimeMapping(void)
return mapping;
}
-/*
- * Build a tuple descriptor for the pg_old_snapshot_time_mapping() SRF.
- */
-static TupleDesc
-MakeOldSnapshotTimeMappingTupleDesc(void)
-{
- TupleDesc tupdesc;
-
- tupdesc = CreateTemplateTupleDesc(NUM_TIME_MAPPING_COLUMNS);
-
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "array_offset",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "end_timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "newest_xmin",
- XIDOID, -1, 0);
-
- return BlessTupleDesc(tupdesc);
-}
-
/*
* Convert one entry from the old snapshot time mapping to a HeapTuple.
*/
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index a95f73ec79..81f262a5f4 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -291,10 +291,8 @@ pg_visibility_map_summary(PG_FUNCTION_ARGS)
ReleaseBuffer(vmbuffer);
relation_close(rel, AccessShareLock);
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "all_visible", INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "all_frozen", INT8OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
values[0] = Int64GetDatum(all_visible);
values[1] = Int64GetDatum(all_frozen);
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 9aa4675cb7..5c30de57ac 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -422,18 +422,8 @@ pg_last_committed_xact(PG_FUNCTION_ARGS)
/* and construct a tuple with our data */
xid = GetLatestCommitTsData(&ts, &nodeid);
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "roident",
- OIDOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (!TransactionIdIsNormal(xid))
{
@@ -476,16 +466,8 @@ pg_xact_commit_timestamp_origin(PG_FUNCTION_ARGS)
found = TransactionIdGetCommitTsData(xid, &ts, &nodeid);
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "roident",
- OIDOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (!found)
{
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index e1191a7564..19b95b8241 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -3373,12 +3373,9 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)
false);
multi->iter = 0;
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "mode",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funccxt->tuple_desc = tupdesc;
funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
funccxt->user_fctx = multi;
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 5017f4451e..cf4e7cb912 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -745,20 +745,9 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* build tupdesc for result tuples */
- /* this had better match pg_prepared_xacts view in system_views.sql */
- tupdesc = CreateTemplateTupleDesc(5);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "transaction",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "gid",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "prepared",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "ownerid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "dbid",
- OIDOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/*
* Collect all the 2PC status information that we will format and send
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 487d5d9cac..bad08270fb 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -356,8 +356,8 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS)
char xlogfilename[MAXFNAMELEN];
Datum values[2];
bool isnull[2];
- TupleDesc resultTupleDesc;
- HeapTuple resultHeapTuple;
+ TupleDesc tupdesc;
+ HeapTuple tuple;
Datum result;
if (RecoveryInProgress())
@@ -367,17 +367,8 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS)
errhint("%s cannot be executed during recovery.",
"pg_walfile_name_offset()")));
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- resultTupleDesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(resultTupleDesc, (AttrNumber) 1, "file_name",
- TEXTOID, -1, 0);
- TupleDescInitEntry(resultTupleDesc, (AttrNumber) 2, "file_offset",
- INT4OID, -1, 0);
-
- resultTupleDesc = BlessTupleDesc(resultTupleDesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/*
* xlogfilename
@@ -400,9 +391,9 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS)
/*
* Tuple jam: Having first prepared your Datums, then squash together
*/
- resultHeapTuple = heap_form_tuple(resultTupleDesc, values, isnull);
+ tuple = heap_form_tuple(tupdesc, values, isnull);
- result = HeapTupleGetDatum(resultHeapTuple);
+ result = HeapTupleGetDatum(tuple);
PG_RETURN_DATUM(result);
}
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index fe97fbf79d..109bdfb33f 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2397,14 +2397,8 @@ pg_get_object_address(PG_FUNCTION_ARGS)
if (relation)
relation_close(relation, AccessShareLock);
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "classid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "objid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "objsubid",
- INT4OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
values[0] = ObjectIdGetDatum(addr.classId);
values[1] = ObjectIdGetDatum(addr.objectId);
@@ -4244,21 +4238,8 @@ pg_identify_object(PG_FUNCTION_ARGS)
address.objectId = objid;
address.objectSubId = objsubid;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(4);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "type",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "schema",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "name",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "identity",
- TEXTOID, -1, 0);
-
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (is_objectclass_supported(address.classId))
{
@@ -4374,19 +4355,8 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
address.objectId = objid;
address.objectSubId = objsubid;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "type",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "object_names",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "object_args",
- TEXTARRAYOID, -1, 0);
-
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* object type, which can never be NULL */
values[0] = CStringGetTextDatum(getObjectTypeDescription(&address, true));
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 59967098b3..913244616a 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -1087,16 +1087,9 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
tables = filter_partitions(tables);
}
- /* Construct a tuple descriptor for the result rows. */
- tupdesc = CreateTemplateTupleDesc(NUM_PUBLICATION_TABLES_ELEM);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "attrs",
- INT2VECTOROID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "qual",
- PG_NODE_TREEOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->user_fctx = (void *) tables;
MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 99c9f91cba..c31c9b891a 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1762,23 +1762,8 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
get_rel_name(relid))));
- tupdesc = CreateTemplateTupleDesc(7);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "cache_size",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "data_type",
- OIDOID, -1, 0);
-
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
memset(isnull, 0, sizeof(isnull));
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 14bb60534f..a82029b8bc 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -46,7 +46,8 @@ typedef struct HeadlineJsonState
static text *headline_json_value(void *_state, char *elem_value, int elem_len);
static void
-tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
+tt_setup_firstcall(FuncCallContext *funcctx, FunctionCallInfo fcinfo,
+ Oid prsid)
{
TupleDesc tupdesc;
MemoryContext oldcontext;
@@ -75,6 +76,12 @@ tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
TEXTOID, -1, 0);
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
+ funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
MemoryContextSwitchTo(oldcontext);
}
@@ -116,7 +123,7 @@ ts_token_type_byid(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
funcctx = SRF_FIRSTCALL_INIT();
- tt_setup_firstcall(funcctx, PG_GETARG_OID(0));
+ tt_setup_firstcall(funcctx, fcinfo, PG_GETARG_OID(0));
}
funcctx = SRF_PERCALL_SETUP();
@@ -139,7 +146,7 @@ ts_token_type_byname(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
prsId = get_ts_parser_oid(textToQualifiedNameList(prsname), false);
- tt_setup_firstcall(funcctx, prsId);
+ tt_setup_firstcall(funcctx, fcinfo, prsId);
}
funcctx = SRF_PERCALL_SETUP();
@@ -164,7 +171,8 @@ typedef struct
static void
-prs_setup_firstcall(FuncCallContext *funcctx, Oid prsid, text *txt)
+prs_setup_firstcall(FuncCallContext *funcctx, FunctionCallInfo fcinfo,
+ Oid prsid, text *txt)
{
TupleDesc tupdesc;
MemoryContext oldcontext;
@@ -209,12 +217,9 @@ prs_setup_firstcall(FuncCallContext *funcctx, Oid prsid, text *txt)
st->cur = 0;
funcctx->user_fctx = (void *) st;
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "tokid",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "token",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
}
@@ -256,7 +261,7 @@ ts_parse_byid(PG_FUNCTION_ARGS)
text *txt = PG_GETARG_TEXT_PP(1);
funcctx = SRF_FIRSTCALL_INIT();
- prs_setup_firstcall(funcctx, PG_GETARG_OID(0), txt);
+ prs_setup_firstcall(funcctx, fcinfo, PG_GETARG_OID(0), txt);
PG_FREE_IF_COPY(txt, 1);
}
@@ -281,7 +286,7 @@ ts_parse_byname(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
prsId = get_ts_parser_oid(textToQualifiedNameList(prsname), false);
- prs_setup_firstcall(funcctx, prsId, txt);
+ prs_setup_firstcall(funcctx, fcinfo, prsId, txt);
}
funcctx = SRF_PERCALL_SETUP();
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index ed1b6a41cf..07a4afd6d0 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -1754,21 +1754,9 @@ aclexplode(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- /*
- * build tupdesc for result tuples (matches out parameters in pg_proc
- * entry)
- */
- tupdesc = CreateTemplateTupleDesc(4);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "grantor",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "grantee",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "privilege_type",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "is_grantable",
- BOOLOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/* allocate memory for user context */
idx = (int *) palloc(sizeof(int[2]));
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index b5b117a8ca..8afda0e5d2 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -4983,19 +4983,10 @@ pg_timezone_abbrevs(PG_FUNCTION_ARGS)
*pindex = 0;
funcctx->user_fctx = (void *) pindex;
- /*
- * build tupdesc for result tuples. This must match this function's
- * pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "abbrev",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "utc_offset",
- INTERVALOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "is_dst",
- BOOLOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
+
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index ab6f67f874..efd948e9ae 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -483,24 +483,8 @@ pg_stat_file(PG_FUNCTION_ARGS)
errmsg("could not stat file \"%s\": %m", filename)));
}
- /*
- * This record type had better match the output parameters declared for me
- * in pg_proc.h.
- */
- tupdesc = CreateTemplateTupleDesc(6);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1,
- "size", INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2,
- "access", TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3,
- "modification", TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4,
- "change", TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5,
- "creation", TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6,
- "isdir", BOOLOID, -1, 0);
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
memset(isnull, false, sizeof(isnull));
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index f9b324efec..5e72c81bf1 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -109,43 +109,9 @@ pg_lock_status(PG_FUNCTION_ARGS)
*/
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- /* build tupdesc for result tuples */
- /* this had better match function's declaration in pg_proc.h */
- tupdesc = CreateTemplateTupleDesc(NUM_LOCK_STATUS_COLUMNS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "locktype",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "database",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "relation",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "page",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "tuple",
- INT2OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "virtualxid",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "transactionid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "classid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "objid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "objsubid",
- INT2OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 11, "virtualtransaction",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 12, "pid",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 13, "mode",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 14, "granted",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 15, "fastpath",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 16, "waitstart",
- TIMESTAMPTZOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/*
* Collect all the locking information that we will format and send
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index d678d28600..7808fbd448 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -427,18 +427,9 @@ pg_get_keywords(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- tupdesc = CreateTemplateTupleDesc(5);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "word",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "catcode",
- CHAROID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "barelabel",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "catdesc",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "baredesc",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
@@ -515,20 +506,8 @@ pg_get_catalog_foreign_keys(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- tupdesc = CreateTemplateTupleDesc(6);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "fktable",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "fkcols",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "pktable",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "pkcols",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "is_array",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "is_opt",
- BOOLOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
funcctx->tuple_desc = BlessTupleDesc(tupdesc);
/*
diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c
index 96b5ae52d2..84518630a5 100644
--- a/src/backend/utils/adt/partitionfuncs.c
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -88,17 +88,9 @@ pg_partition_tree(PG_FUNCTION_ARGS)
*/
partitions = find_all_inheritors(rootrelid, AccessShareLock, NULL);
- tupdesc = CreateTemplateTupleDesc(PG_PARTITION_TREE_COLS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "isleaf",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "level",
- INT4OID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/* The only state we need is the partition list */
funcctx->user_fctx = (void *) partitions;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 04a5a99002..060e1f606e 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1210,27 +1210,8 @@ pg_stat_get_wal(PG_FUNCTION_ARGS)
PgStat_WalStats *wal_stats;
/* Initialise attributes information in the tuple descriptor */
- tupdesc = CreateTemplateTupleDesc(PG_STAT_GET_WAL_COLS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "wal_records",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "wal_fpi",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "wal_bytes",
- NUMERICOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "wal_buffers_full",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "wal_write",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "wal_sync",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "wal_write_time",
- FLOAT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "wal_sync_time",
- FLOAT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "stats_reset",
- TIMESTAMPTZOID, -1, 0);
-
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* Get statistics about WAL activity */
wal_stats = pgstat_fetch_stat_wal();
@@ -1657,23 +1638,8 @@ pg_stat_get_archiver(PG_FUNCTION_ARGS)
PgStat_ArchiverStats *archiver_stats;
/* Initialise attributes information in the tuple descriptor */
- tupdesc = CreateTemplateTupleDesc(7);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "archived_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "last_archived_wal",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "last_archived_time",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "failed_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "last_failed_wal",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "last_failed_time",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "stats_reset",
- TIMESTAMPTZOID, -1, 0);
-
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* Get statistics about the archiver process */
archiver_stats = pgstat_fetch_stat_archiver();
@@ -1727,28 +1693,8 @@ pg_stat_get_replication_slot(PG_FUNCTION_ARGS)
PgStat_StatReplSlotEntry allzero;
/* Initialise attributes information in the tuple descriptor */
- tupdesc = CreateTemplateTupleDesc(PG_STAT_GET_REPLICATION_SLOT_COLS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "slot_name",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "spill_txns",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "spill_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "spill_bytes",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "stream_txns",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "stream_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "stream_bytes",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "total_txns",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "total_bytes",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "stats_reset",
- TIMESTAMPTZOID, -1, 0);
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
namestrcpy(&slotname, text_to_cstring(slotname_text));
slotent = pgstat_fetch_replslot(slotname);
@@ -1800,16 +1746,8 @@ pg_stat_get_subscription_stats(PG_FUNCTION_ARGS)
subentry = pgstat_fetch_stat_subscription(subid);
/* Initialise attributes information in the tuple descriptor */
- tupdesc = CreateTemplateTupleDesc(PG_STAT_GET_SUBSCRIPTION_STATS_COLS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "subid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "apply_error_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "sync_error_count",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "stats_reset",
- TIMESTAMPTZOID, -1, 0);
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (!subentry)
{
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index f7c1e3d6d6..caeb85b4ca 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -647,7 +647,9 @@ tsvector_unnest(PG_FUNCTION_ARGS)
INT2ARRAYOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "weights",
TEXTARRAYOID, -1, 0);
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->user_fctx = PG_GETARG_TSVECTOR_COPY(0);
@@ -2302,14 +2304,9 @@ ts_setup_firstcall(FunctionCallInfo fcinfo, FuncCallContext *funcctx,
}
Assert(stat->stackpos <= stat->maxdepth);
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "word",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "ndoc",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "nentry",
- INT4OID, -1, 0);
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c
index 23da603fe7..8909192f0f 100644
--- a/src/backend/utils/misc/guc_funcs.c
+++ b/src/backend/utils/misc/guc_funcs.c
@@ -873,45 +873,9 @@ show_all_settings(PG_FUNCTION_ARGS)
*/
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- /*
- * need a tuple descriptor representing NUM_PG_SETTINGS_ATTS columns
- * of the appropriate types
- */
- tupdesc = CreateTemplateTupleDesc(NUM_PG_SETTINGS_ATTS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "setting",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "unit",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "category",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "short_desc",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "extra_desc",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "context",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "vartype",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "source",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "min_val",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 11, "max_val",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 12, "enumvals",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 13, "boot_val",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 14, "reset_val",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 15, "sourcefile",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 16, "sourceline",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 17, "pending_restart",
- BOOLOID, -1, 0);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/*
* Generate attribute metadata needed later to produce tuples from raw
diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 781f8b8758..0703892ed4 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -38,20 +38,8 @@ pg_control_system(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(4);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "pg_control_version",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "catalog_version_no",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "system_identifier",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "pg_control_last_modified",
- TIMESTAMPTZOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* read the control file */
ControlFile = get_controlfile(DataDir, &crc_ok);
@@ -88,48 +76,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
char xlogfilename[MAXFNAMELEN];
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(18);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "checkpoint_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "redo_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "redo_wal_file",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "timeline_id",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "prev_timeline_id",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "full_page_writes",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "next_xid",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "next_oid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "next_multixact_id",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "next_multi_offset",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 11, "oldest_xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 12, "oldest_xid_dbid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 13, "oldest_active_xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 14, "oldest_multi_xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 15, "oldest_multi_dbid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 16, "oldest_commit_ts_xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 17, "newest_commit_ts_xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 18, "checkpoint_time",
- TIMESTAMPTZOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* Read the control file. */
ControlFile = get_controlfile(DataDir, &crc_ok);
@@ -217,22 +165,8 @@ pg_control_recovery(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(5);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "min_recovery_end_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "min_recovery_end_timeline",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "backup_start_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "backup_end_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "end_of_backup_record_required",
- BOOLOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* read the control file */
ControlFile = get_controlfile(DataDir, &crc_ok);
@@ -270,34 +204,8 @@ pg_control_init(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(11);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "max_data_alignment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "database_block_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "blocks_per_segment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "wal_block_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "bytes_per_wal_segment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "max_identifier_length",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "max_index_columns",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "max_toast_chunk_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "large_object_chunk_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "float8_pass_by_value",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 11, "data_page_checksum_version",
- INT4OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* read the control file */
ControlFile = get_controlfile(DataDir, &crc_ok);
diff --git a/src/test/modules/test_predtest/test_predtest.c b/src/test/modules/test_predtest/test_predtest.c
index 2ce88cb624..abb66180bc 100644
--- a/src/test/modules/test_predtest/test_predtest.c
+++ b/src/test/modules/test_predtest/test_predtest.c
@@ -185,24 +185,8 @@ test_predtest(PG_FUNCTION_ARGS)
if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
- tupdesc = CreateTemplateTupleDesc(8);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1,
- "strong_implied_by", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2,
- "weak_implied_by", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3,
- "strong_refuted_by", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4,
- "weak_refuted_by", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5,
- "s_i_holds", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6,
- "w_i_holds", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7,
- "s_r_holds", BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8,
- "w_r_holds", BOOLOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
values[0] = BoolGetDatum(strong_implied_by);
values[1] = BoolGetDatum(weak_implied_by);
--
2.34.1
v1-0002-Remove-unnecessary-BlessTupleDesc-after-get_call_.patchapplication/x-patch; name=v1-0002-Remove-unnecessary-BlessTupleDesc-after-get_call_.patchDownload
From 55ca110d878351d30e7cc3a145cc403669efd0e3 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 13 Dec 2022 07:04:42 +0000
Subject: [PATCH v1] Remove unnecessary BlessTupleDesc() after
get_call_result_type()
get_call_result_type() already does what BlessTupleDesc() intends
to do for the returned tuple descriptor. Hence, it is unnecessary
to call BlessTupleDesc() it right after get_call_result_type().
---
contrib/hstore/hstore_op.c | 6 +-----
contrib/pageinspect/brinfuncs.c | 1 -
contrib/pageinspect/btreefuncs.c | 2 --
contrib/pageinspect/hashfuncs.c | 2 --
contrib/sslinfo/sslinfo.c | 6 +-----
src/backend/statistics/mcv.c | 1 -
src/test/regress/regress.c | 1 -
7 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/contrib/hstore/hstore_op.c b/contrib/hstore/hstore_op.c
index 0d4ec16d1e..2b3a1e6a2f 100644
--- a/contrib/hstore/hstore_op.c
+++ b/contrib/hstore/hstore_op.c
@@ -862,13 +862,9 @@ setup_firstcall(FuncCallContext *funcctx, HStore *hs,
if (fcinfo)
{
- TupleDesc tupdesc;
-
/* Build a tuple descriptor for our result type */
- if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ if (get_call_result_type(fcinfo, NULL, &funcctx->tuple_desc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
}
MemoryContextSwitchTo(oldcontext);
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 12a7217038..b812cdeee3 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -348,7 +348,6 @@ brin_metapage_info(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupdesc = BlessTupleDesc(tupdesc);
/* Extract values from the metapage */
meta = (BrinMetaPageData *) PageGetContents(page);
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 9375d55e14..5e083a6f9c 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -537,7 +537,6 @@ bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
uargs->tupd = tupleDesc;
@@ -653,7 +652,6 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
uargs->tupd = tupleDesc;
diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c
index 81815392d7..e1364c5518 100644
--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -259,7 +259,6 @@ hash_page_stats(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
j = 0;
values[j++] = Int32GetDatum(stat.live_items);
@@ -334,7 +333,6 @@ hash_page_items(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
fctx->attinmeta = TupleDescGetAttInMetadata(tupleDesc);
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index 5fd46b9874..6e313ced5e 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -370,9 +370,6 @@ ssl_extension_info(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
-
- TupleDesc tupdesc;
-
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
@@ -385,11 +382,10 @@ ssl_extension_info(PG_FUNCTION_ARGS)
fctx = (SSLExtensionInfoContext *) palloc(sizeof(SSLExtensionInfoContext));
/* Construct tuple descriptor */
- if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ if (get_call_result_type(fcinfo, NULL, &fctx->tupdesc) != TYPEFUNC_COMPOSITE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("function returning record called in context that cannot accept type record")));
- fctx->tupdesc = BlessTupleDesc(tupdesc);
/* Set max_calls as a count of extensions in certificate */
max_calls = cert != NULL ? X509_get_ext_count(cert) : 0;
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index f5a7c31272..592c76fea7 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1371,7 +1371,6 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("function returning record called in context "
"that cannot accept type record")));
- tupdesc = BlessTupleDesc(tupdesc);
/*
* generate attribute metadata needed later to produce tuples from raw
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 2977045cc7..4d5e3e0918 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -1133,7 +1133,6 @@ test_enc_conversion(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupdesc = BlessTupleDesc(tupdesc);
srclen = VARSIZE_ANY_EXHDR(string);
src = VARDATA_ANY(string);
--
2.34.1
On Tue, Dec 13, 2022 at 01:06:48PM +0530, Bharath Rupireddy wrote:
A review comment in another thread [1] by Michael Paquier about the
usage of get_call_result_type() instead of explicit building of
TupleDesc made me think about using it more widely. Actually, the
get_call_result_type() looks at the function definitions to figure the
column names and build the required TupleDesc, usage of which avoids
duplication of the column names between pg_proc.dat/function
definitions and source code. Also, it saves a good number of LOC ~415
[2] and the size of all the object files put together gets reduced by
~4MB, which means, the postgres binary becomes leaner by ~4MB [3]. I'm
attaching a patch for these changes.
I have wanted to look at that when poking at the interface for
materialized SRFs but lacked of steam back then. Even after this
change, we still have coverage for CreateTemplateTupleDesc() and
TupleDescInitEntry() through the GUCs/SHOW or even WAL sender, so the
coverage does not worry me much. Backpatch conflicts may be a point
of contention, but that's pretty much in the same spirit as
SetSingleFuncCall()/InitMaterializedSRF().
All in that, +1 (still need to check in details what you have here,
looks rather fine at quick glance).
--
Michael
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
A review comment in another thread [1] by Michael Paquier about the
usage of get_call_result_type() instead of explicit building of
TupleDesc made me think about using it more widely. Actually, the
get_call_result_type() looks at the function definitions to figure the
column names and build the required TupleDesc, usage of which avoids
duplication of the column names between pg_proc.dat/function
definitions and source code. Also, it saves a good number of LOC ~415
[2] and the size of all the object files put together gets reduced by
~4MB, which means, the postgres binary becomes leaner by ~4MB [3].
Saving code is nice, but I'd assume the result is slower, because
get_call_result_type has to do a pretty substantial amount of work
to get the data to construct the tupdesc from. Have you tried to
quantify how much overhead this'd add? Which of these functions
can we safely consider to be non-performance-critical?
regards, tom lane
On Tue, Dec 13, 2022 at 9:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
A review comment in another thread [1] by Michael Paquier about the
usage of get_call_result_type() instead of explicit building of
TupleDesc made me think about using it more widely. Actually, the
get_call_result_type() looks at the function definitions to figure the
column names and build the required TupleDesc, usage of which avoids
duplication of the column names between pg_proc.dat/function
definitions and source code. Also, it saves a good number of LOC ~415
[2] and the size of all the object files put together gets reduced by
~4MB, which means, the postgres binary becomes leaner by ~4MB [3].Saving code is nice, but I'd assume the result is slower, because
get_call_result_type has to do a pretty substantial amount of work
to get the data to construct the tupdesc from. Have you tried to
quantify how much overhead this'd add? Which of these functions
can we safely consider to be non-performance-critical?
AFAICS, most of these functions have no direct source code callers,
they're user-facing functions and not in a hot code path. I measured
the test times of these functions and I don't see much difference [1]pg_old_snapshot_time_mapping() - an extension function with no internal source code callers, no test coverage. pg_visibility_map_summary() - an extension function with no internal source code callers, test coverage exists, test times on HEAD:25 ms PATCHED:25 ms pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no internal source code callers, test coverage exists, test times on HEAD:10 ms PATCHED:10 ms pg_get_multixact_members() - no internal source code callers, no test coverage. pg_prepared_xact() - no internal source code callers, test coverage exists, test times on HEAD:50 ms, subscription 108 wallclock secs, recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock secs, recovery 112 wallclock secs pg_walfile_name_offset() - no internal source code callers, no test coverage. pg_get_object_address() - no internal source code callers, test coverage exists, test times on HEAD:66 ms PATCHED:60 ms pg_identify_object() - no internal source code callers, test coverage exists, test times on HEAD:17 ms PATCHED:18 ms pg_identify_object_as_address() - no internal source code callers, test coverage exists, test times on HEAD:66 ms PATCHED:60 ms pg_get_publication_tables() - internal source code callers exist, test coverage exists, test times on HEAD:159 ms, subscription 108 wallclock secs PATCHED:167 ms, subscription 110 wallclock secs pg_sequence_parameters() - no internal source code callers, test coverage exists, test times on HEAD:96 ms PATCHED:98 ms ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and ts_parse_byname() - internal source code callers exists, test coverage exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs PATCHED:186 ms, pg_dump 10 wallclock secs aclexplode() - internal callers exists information_schema.sql, indirect test coverage exists. pg_timezone_abbrevs() - no internal source code callers, test coverage exists, test times on HEAD:40 ms PATCHED:36 ms pg_stat_file() - no internal source code callers, test coverage exists, test times on HEAD:42 ms PATCHED:46 ms pg_lock_status() - no internal source code callers, test coverage exists, test times on HEAD:16 ms PATCHED:22 ms pg_get_keywords() - no internal source code callers, test coverage exists, test times on HEAD:129 ms PATCHED:130 ms pg_get_catalog_foreign_keys() - no internal source code callers, test coverage exists, test times on HEAD:114 ms PATCHED:111 ms pg_partition_tree() - no internal source code callers, test coverage exists, test times on HEAD:30 ms PATCHED:32 ms pg_stat_get_wal(), pg_stat_get_archiver() and pg_stat_get_replication_slot() - no internal source code callers, test coverage exists, test times on HEAD:479 ms PATCHED:483 ms pg_stat_get_subscription_stats() - no internal source code callers, test coverage exists, test times on HEAD:subscription 108 wallclock secs PATCHED:subscription 110 wallclock secs tsvector_unnest() - no internal source code callers, test coverage exists, test times on HEAD:26 ms PATCHED:26 ms ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms PATCHED:186 ms show_all_settings(), pg_control_system(), pg_control_checkpoint(), pg_control_recovery() and pg_control_init() - test coverage exists, test times on HEAD:42 ms PATCHED:44 ms test_predtest() - no internal source code callers, test coverage exists, test times on HEAD:18 ms PATCHED:18 ms.
[1]: pg_old_snapshot_time_mapping() - an extension function with no internal source code callers, no test coverage. pg_visibility_map_summary() - an extension function with no internal source code callers, test coverage exists, test times on HEAD:25 ms PATCHED:25 ms pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no internal source code callers, test coverage exists, test times on HEAD:10 ms PATCHED:10 ms pg_get_multixact_members() - no internal source code callers, no test coverage. pg_prepared_xact() - no internal source code callers, test coverage exists, test times on HEAD:50 ms, subscription 108 wallclock secs, recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock secs, recovery 112 wallclock secs pg_walfile_name_offset() - no internal source code callers, no test coverage. pg_get_object_address() - no internal source code callers, test coverage exists, test times on HEAD:66 ms PATCHED:60 ms pg_identify_object() - no internal source code callers, test coverage exists, test times on HEAD:17 ms PATCHED:18 ms pg_identify_object_as_address() - no internal source code callers, test coverage exists, test times on HEAD:66 ms PATCHED:60 ms pg_get_publication_tables() - internal source code callers exist, test coverage exists, test times on HEAD:159 ms, subscription 108 wallclock secs PATCHED:167 ms, subscription 110 wallclock secs pg_sequence_parameters() - no internal source code callers, test coverage exists, test times on HEAD:96 ms PATCHED:98 ms ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and ts_parse_byname() - internal source code callers exists, test coverage exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs PATCHED:186 ms, pg_dump 10 wallclock secs aclexplode() - internal callers exists information_schema.sql, indirect test coverage exists. pg_timezone_abbrevs() - no internal source code callers, test coverage exists, test times on HEAD:40 ms PATCHED:36 ms pg_stat_file() - no internal source code callers, test coverage exists, test times on HEAD:42 ms PATCHED:46 ms pg_lock_status() - no internal source code callers, test coverage exists, test times on HEAD:16 ms PATCHED:22 ms pg_get_keywords() - no internal source code callers, test coverage exists, test times on HEAD:129 ms PATCHED:130 ms pg_get_catalog_foreign_keys() - no internal source code callers, test coverage exists, test times on HEAD:114 ms PATCHED:111 ms pg_partition_tree() - no internal source code callers, test coverage exists, test times on HEAD:30 ms PATCHED:32 ms pg_stat_get_wal(), pg_stat_get_archiver() and pg_stat_get_replication_slot() - no internal source code callers, test coverage exists, test times on HEAD:479 ms PATCHED:483 ms pg_stat_get_subscription_stats() - no internal source code callers, test coverage exists, test times on HEAD:subscription 108 wallclock secs PATCHED:subscription 110 wallclock secs tsvector_unnest() - no internal source code callers, test coverage exists, test times on HEAD:26 ms PATCHED:26 ms ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms PATCHED:186 ms show_all_settings(), pg_control_system(), pg_control_checkpoint(), pg_control_recovery() and pg_control_init() - test coverage exists, test times on HEAD:42 ms PATCHED:44 ms test_predtest() - no internal source code callers, test coverage exists, test times on HEAD:18 ms PATCHED:18 ms
pg_old_snapshot_time_mapping() - an extension function with no
internal source code callers, no test coverage.
pg_visibility_map_summary() - an extension function with no internal
source code callers, test coverage exists, test times on HEAD:25 ms
PATCHED:25 ms
pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no
internal source code callers, test coverage exists, test times on
HEAD:10 ms PATCHED:10 ms
pg_get_multixact_members() - no internal source code callers, no test coverage.
pg_prepared_xact() - no internal source code callers, test coverage
exists, test times on HEAD:50 ms, subscription 108 wallclock secs,
recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock
secs, recovery 112 wallclock secs
pg_walfile_name_offset() - no internal source code callers, no test coverage.
pg_get_object_address() - no internal source code callers, test
coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_identify_object() - no internal source code callers, test coverage
exists, test times on HEAD:17 ms PATCHED:18 ms
pg_identify_object_as_address() - no internal source code callers,
test coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_get_publication_tables() - internal source code callers exist, test
coverage exists, test times on HEAD:159 ms, subscription 108 wallclock
secs PATCHED:167 ms, subscription 110 wallclock secs
pg_sequence_parameters() - no internal source code callers, test
coverage exists, test times on HEAD:96 ms PATCHED:98 ms
ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and
ts_parse_byname() - internal source code callers exists, test coverage
exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs
PATCHED:186 ms, pg_dump 10 wallclock secs
aclexplode() - internal callers exists information_schema.sql,
indirect test coverage exists.
pg_timezone_abbrevs() - no internal source code callers, test coverage
exists, test times on HEAD:40 ms PATCHED:36 ms
pg_stat_file() - no internal source code callers, test coverage
exists, test times on HEAD:42 ms PATCHED:46 ms
pg_lock_status() - no internal source code callers, test coverage
exists, test times on HEAD:16 ms PATCHED:22 ms
pg_get_keywords() - no internal source code callers, test coverage
exists, test times on HEAD:129 ms PATCHED:130 ms
pg_get_catalog_foreign_keys() - no internal source code callers, test
coverage exists, test times on HEAD:114 ms PATCHED:111 ms
pg_partition_tree() - no internal source code callers, test coverage
exists, test times on HEAD:30 ms PATCHED:32 ms
pg_stat_get_wal(), pg_stat_get_archiver() and
pg_stat_get_replication_slot() - no internal source code callers, test
coverage exists, test times on HEAD:479 ms PATCHED:483 ms
pg_stat_get_subscription_stats() - no internal source code callers,
test coverage exists, test times on HEAD:subscription 108 wallclock
secs PATCHED:subscription 110 wallclock secs
tsvector_unnest() - no internal source code callers, test coverage
exists, test times on HEAD:26 ms PATCHED:26 ms
ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms
PATCHED:186 ms
show_all_settings(), pg_control_system(), pg_control_checkpoint(),
pg_control_recovery() and pg_control_init() - test coverage exists,
test times on HEAD:42 ms PATCHED:44 ms
test_predtest() - no internal source code callers, test coverage
exists, test times on HEAD:18 ms PATCHED:18 ms
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Wed, Dec 14, 2022 at 11:14:59AM +0530, Bharath Rupireddy wrote:
AFAICS, most of these functions have no direct source code callers,
they're user-facing functions and not in a hot code path. I measured
the test times of these functions and I don't see much difference [1].
Thanks for the summary. It looks like your tests involve single
runs. What is the difference in run-time when invoking this
repeatedly with a large generate_series() for example when few or no
tuples are returned? Do you see a difference in perf profile? Some
of the functions could have their time mostly eaten while looking at
the syscache on repeated calls, but you could see the actual work this
involves with a dummy function that returns a large number of
attributes on a single record in the worst case possible?
Separating things into two buckets..
[1]
pg_old_snapshot_time_mapping() - an extension function with no
internal source code callers, no test coverage.
pg_visibility_map_summary() - an extension function with no internal
source code callers, test coverage exists, test times on HEAD:25 ms
PATCHED:25 ms
pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no
internal source code callers, test coverage exists, test times on
HEAD:10 ms PATCHED:10 ms> pg_get_multixact_members() - no internal source code callers, no test coverage.
pg_control_recovery() and pg_control_init() - test coverage exists,
test times on HEAD:42 ms PATCHED:44 ms
pg_identify_object() - no internal source code callers, test coverage
exists, test times on HEAD:17 ms PATCHED:18 ms
pg_identify_object_as_address() - no internal source code callers,
test coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_get_object_address() - no internal source code callers, test
coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_sequence_parameters() - no internal source code callers, test
coverage exists, test times on HEAD:96 ms PATCHED:98 ms
ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and
ts_parse_byname() - internal source code callers exists, test coverage
exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs
PATCHED:186 ms, pg_dump 10 wallclock secs
pg_get_keywords() - no internal source code callers, test coverage
exists, test times on HEAD:129 ms PATCHED:130 ms
pg_get_catalog_foreign_keys() - no internal source code callers, test
coverage exists, test times on HEAD:114 ms PATCHED:111 ms
tsvector_unnest() - no internal source code callers, test coverage
exists, test times on HEAD:26 ms PATCHED:26 ms
ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms
PATCHED:186 ms
pg_partition_tree() - no internal source code callers, test coverage
exists, test times on HEAD:30 ms PATCHED:32 ms
pg_timezone_abbrevs() - no internal source code callers, test coverage
exists, test times on HEAD:40 ms PATCHED:36 ms
These ones don't worry me much, TBH.
pg_stat_get_wal(), pg_stat_get_archiver() and
pg_stat_get_replication_slot() - no internal source code callers, test
coverage exists, test times on HEAD:479 ms PATCHED:483 ms
pg_prepared_xact() - no internal source code callers, test coverage
exists, test times on HEAD:50 ms, subscription 108 wallclock secs,
recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock
secs, recovery 112 wallclock secs
show_all_settings(), pg_control_system(), pg_control_checkpoint(),
test_predtest() - no internal source code callers, test coverage
exists, test times on HEAD:18 ms PATCHED:18 ms
pg_walfile_name_offset() - no internal source code callers, no test coverage.
aclexplode() - internal callers exists information_schema.sql,
indirect test coverage exists.
pg_stat_file() - no internal source code callers, test coverage
exists, test times on HEAD:42 ms PATCHED:46 ms
pg_get_publication_tables() - internal source code callers exist, test
coverage exists, test times on HEAD:159 ms, subscription 108 wallclock
secs PATCHED:167 ms, subscription 110 wallclock secs
pg_lock_status() - no internal source code callers, test coverage
exists, test times on HEAD:16 ms PATCHED:22 ms
pg_stat_get_subscription_stats() - no internal source code callers,
test coverage exists, test times on HEAD:subscription 108 wallclock
secs PATCHED:subscription 110 wallclock secs
These ones could be involved in monitoring queries run on a periodic
basis.
--
Michael
On Thu, Dec 15, 2022 at 11:41 AM Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Dec 14, 2022 at 11:14:59AM +0530, Bharath Rupireddy wrote:
AFAICS, most of these functions have no direct source code callers,
they're user-facing functions and not in a hot code path. I measured
the test times of these functions and I don't see much difference [1].Thanks for the summary. It looks like your tests involve single
runs. What is the difference in run-time when invoking this
repeatedly with a large generate_series() for example when few or no
tuples are returned? Do you see a difference in perf profile? Some
of the functions could have their time mostly eaten while looking at
the syscache on repeated calls, but you could see the actual work this
involves with a dummy function that returns a large number of
attributes on a single record in the worst case possible?
Thanks. Yes, using get_call_result_type() for a function that gets
called repeatedly does have some cost as the comment around
get_call_result_type() says - I found in my testing that
get_call_result_type() does seem to cost 45% increase in execution
times over quick iterations of a function returning a single row with
36 columns.
Separating things into two buckets..
[1]
pg_old_snapshot_time_mapping() - an extension function with no
internal source code callers, no test coverage.
pg_visibility_map_summary() - an extension function with no internal
source code callers, test coverage exists, test times on HEAD:25 ms
PATCHED:25 ms
pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no
internal source code callers, test coverage exists, test times on
HEAD:10 ms PATCHED:10 ms> pg_get_multixact_members() - no internal source code callers, no test coverage.
pg_control_recovery() and pg_control_init() - test coverage exists,
test times on HEAD:42 ms PATCHED:44 ms
pg_identify_object() - no internal source code callers, test coverage
exists, test times on HEAD:17 ms PATCHED:18 ms
pg_identify_object_as_address() - no internal source code callers,
test coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_get_object_address() - no internal source code callers, test
coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_sequence_parameters() - no internal source code callers, test
coverage exists, test times on HEAD:96 ms PATCHED:98 ms
ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and
ts_parse_byname() - internal source code callers exists, test coverage
exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs
PATCHED:186 ms, pg_dump 10 wallclock secs
pg_get_keywords() - no internal source code callers, test coverage
exists, test times on HEAD:129 ms PATCHED:130 ms
pg_get_catalog_foreign_keys() - no internal source code callers, test
coverage exists, test times on HEAD:114 ms PATCHED:111 ms
tsvector_unnest() - no internal source code callers, test coverage
exists, test times on HEAD:26 ms PATCHED:26 ms
ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms
PATCHED:186 ms
pg_partition_tree() - no internal source code callers, test coverage
exists, test times on HEAD:30 ms PATCHED:32 ms
pg_timezone_abbrevs() - no internal source code callers, test coverage
exists, test times on HEAD:40 ms PATCHED:36 msThese ones don't worry me much, TBH.
pg_stat_get_wal(), pg_stat_get_archiver() and
pg_stat_get_replication_slot() - no internal source code callers, test
coverage exists, test times on HEAD:479 ms PATCHED:483 ms
pg_prepared_xact() - no internal source code callers, test coverage
exists, test times on HEAD:50 ms, subscription 108 wallclock secs,
recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock
secs, recovery 112 wallclock secs
show_all_settings(), pg_control_system(), pg_control_checkpoint(),
test_predtest() - no internal source code callers, test coverage
exists, test times on HEAD:18 ms PATCHED:18 ms
pg_walfile_name_offset() - no internal source code callers, no test coverage.
aclexplode() - internal callers exists information_schema.sql,
indirect test coverage exists.
pg_stat_file() - no internal source code callers, test coverage
exists, test times on HEAD:42 ms PATCHED:46 ms
pg_get_publication_tables() - internal source code callers exist, test
coverage exists, test times on HEAD:159 ms, subscription 108 wallclock
secs PATCHED:167 ms, subscription 110 wallclock secs
pg_lock_status() - no internal source code callers, test coverage
exists, test times on HEAD:16 ms PATCHED:22 ms
pg_stat_get_subscription_stats() - no internal source code callers,
test coverage exists, test times on HEAD:subscription 108 wallclock
secs PATCHED:subscription 110 wallclock secsThese ones could be involved in monitoring queries run on a periodic
basis.
I agree with the bucketization. Please see the attached patches. 0001
- gets rid of explicit tuple desc creation using
get_call_result_type() for functions thought to be not-so-frequently
called. 0002 - gets rid of an unnecessary call to BlessTupleDesc()
after get_call_result_type().
Please find the attached patches.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Use-get_call_result_type-for-not-so-frequently-ca.patchapplication/x-patch; name=v2-0001-Use-get_call_result_type-for-not-so-frequently-ca.patchDownload
From 87b29e5598e391002e6bf684c85bfdfde85a87b3 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 19 Dec 2022 12:56:34 +0000
Subject: [PATCH v2] Use get_call_result_type() for not-so-frequently called
functions
Usage of get_call_result_type() in more places avoids explicit
creation of tuple descriptor which requires matching column
names from pg_proc.dat/function definitions. It saves some code
too.
However, creating tuple descriptor explicitly still fares better
for the functions that may get called so frequently as
get_call_result_type() comes with a bit of cost (it looks at
system catalogs for fetching column names).
Hence, this commit uses get_call_result_type() for the functions
that may not get called so frequently.
---
contrib/old_snapshot/time_mapping.c | 26 +++-----------
contrib/pg_visibility/pg_visibility.c | 6 ++--
src/backend/access/transam/commit_ts.c | 26 +++-----------
src/backend/access/transam/multixact.c | 9 ++---
src/backend/catalog/objectaddress.c | 42 ++++------------------
src/backend/commands/sequence.c | 19 ++--------
src/backend/tsearch/wparser.c | 29 ++++++++-------
src/backend/utils/adt/datetime.c | 17 +++------
src/backend/utils/adt/misc.c | 31 +++-------------
src/backend/utils/adt/partitionfuncs.c | 14 ++------
src/backend/utils/adt/tsvector_op.c | 15 ++++----
src/backend/utils/misc/pg_controldata.c | 48 +++----------------------
12 files changed, 60 insertions(+), 222 deletions(-)
diff --git a/contrib/old_snapshot/time_mapping.c b/contrib/old_snapshot/time_mapping.c
index 2d8cb742c3..97efccddbb 100644
--- a/contrib/old_snapshot/time_mapping.c
+++ b/contrib/old_snapshot/time_mapping.c
@@ -38,7 +38,6 @@ PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(pg_old_snapshot_time_mapping);
static OldSnapshotTimeMapping *GetOldSnapshotTimeMapping(void);
-static TupleDesc MakeOldSnapshotTimeMappingTupleDesc(void);
static HeapTuple MakeOldSnapshotTimeMappingTuple(TupleDesc tupdesc,
OldSnapshotTimeMapping *mapping);
@@ -54,12 +53,15 @@ pg_old_snapshot_time_mapping(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
MemoryContext oldcontext;
+ TupleDesc tupdesc;
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
mapping = GetOldSnapshotTimeMapping();
funcctx->user_fctx = mapping;
- funcctx->tuple_desc = MakeOldSnapshotTimeMappingTupleDesc();
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
MemoryContextSwitchTo(oldcontext);
}
@@ -101,26 +103,6 @@ GetOldSnapshotTimeMapping(void)
return mapping;
}
-/*
- * Build a tuple descriptor for the pg_old_snapshot_time_mapping() SRF.
- */
-static TupleDesc
-MakeOldSnapshotTimeMappingTupleDesc(void)
-{
- TupleDesc tupdesc;
-
- tupdesc = CreateTemplateTupleDesc(NUM_TIME_MAPPING_COLUMNS);
-
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "array_offset",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "end_timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "newest_xmin",
- XIDOID, -1, 0);
-
- return BlessTupleDesc(tupdesc);
-}
-
/*
* Convert one entry from the old snapshot time mapping to a HeapTuple.
*/
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index a95f73ec79..81f262a5f4 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -291,10 +291,8 @@ pg_visibility_map_summary(PG_FUNCTION_ARGS)
ReleaseBuffer(vmbuffer);
relation_close(rel, AccessShareLock);
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "all_visible", INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "all_frozen", INT8OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
values[0] = Int64GetDatum(all_visible);
values[1] = Int64GetDatum(all_frozen);
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 9aa4675cb7..5c30de57ac 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -422,18 +422,8 @@ pg_last_committed_xact(PG_FUNCTION_ARGS)
/* and construct a tuple with our data */
xid = GetLatestCommitTsData(&ts, &nodeid);
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "roident",
- OIDOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (!TransactionIdIsNormal(xid))
{
@@ -476,16 +466,8 @@ pg_xact_commit_timestamp_origin(PG_FUNCTION_ARGS)
found = TransactionIdGetCommitTsData(xid, &ts, &nodeid);
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "timestamp",
- TIMESTAMPTZOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "roident",
- OIDOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (!found)
{
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index e1191a7564..19b95b8241 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -3373,12 +3373,9 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)
false);
multi->iter = 0;
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "xid",
- XIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "mode",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funccxt->tuple_desc = tupdesc;
funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
funccxt->user_fctx = multi;
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index fe97fbf79d..109bdfb33f 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2397,14 +2397,8 @@ pg_get_object_address(PG_FUNCTION_ARGS)
if (relation)
relation_close(relation, AccessShareLock);
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "classid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "objid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "objsubid",
- INT4OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
values[0] = ObjectIdGetDatum(addr.classId);
values[1] = ObjectIdGetDatum(addr.objectId);
@@ -4244,21 +4238,8 @@ pg_identify_object(PG_FUNCTION_ARGS)
address.objectId = objid;
address.objectSubId = objsubid;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(4);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "type",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "schema",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "name",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "identity",
- TEXTOID, -1, 0);
-
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
if (is_objectclass_supported(address.classId))
{
@@ -4374,19 +4355,8 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
address.objectId = objid;
address.objectSubId = objsubid;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "type",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "object_names",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "object_args",
- TEXTARRAYOID, -1, 0);
-
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* object type, which can never be NULL */
values[0] = CStringGetTextDatum(getObjectTypeDescription(&address, true));
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 99c9f91cba..c31c9b891a 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1762,23 +1762,8 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
get_rel_name(relid))));
- tupdesc = CreateTemplateTupleDesc(7);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "cache_size",
- INT8OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "data_type",
- OIDOID, -1, 0);
-
- BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
memset(isnull, 0, sizeof(isnull));
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 14bb60534f..a82029b8bc 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -46,7 +46,8 @@ typedef struct HeadlineJsonState
static text *headline_json_value(void *_state, char *elem_value, int elem_len);
static void
-tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
+tt_setup_firstcall(FuncCallContext *funcctx, FunctionCallInfo fcinfo,
+ Oid prsid)
{
TupleDesc tupdesc;
MemoryContext oldcontext;
@@ -75,6 +76,12 @@ tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
TEXTOID, -1, 0);
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
+ funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
MemoryContextSwitchTo(oldcontext);
}
@@ -116,7 +123,7 @@ ts_token_type_byid(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
funcctx = SRF_FIRSTCALL_INIT();
- tt_setup_firstcall(funcctx, PG_GETARG_OID(0));
+ tt_setup_firstcall(funcctx, fcinfo, PG_GETARG_OID(0));
}
funcctx = SRF_PERCALL_SETUP();
@@ -139,7 +146,7 @@ ts_token_type_byname(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
prsId = get_ts_parser_oid(textToQualifiedNameList(prsname), false);
- tt_setup_firstcall(funcctx, prsId);
+ tt_setup_firstcall(funcctx, fcinfo, prsId);
}
funcctx = SRF_PERCALL_SETUP();
@@ -164,7 +171,8 @@ typedef struct
static void
-prs_setup_firstcall(FuncCallContext *funcctx, Oid prsid, text *txt)
+prs_setup_firstcall(FuncCallContext *funcctx, FunctionCallInfo fcinfo,
+ Oid prsid, text *txt)
{
TupleDesc tupdesc;
MemoryContext oldcontext;
@@ -209,12 +217,9 @@ prs_setup_firstcall(FuncCallContext *funcctx, Oid prsid, text *txt)
st->cur = 0;
funcctx->user_fctx = (void *) st;
- tupdesc = CreateTemplateTupleDesc(2);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "tokid",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "token",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
}
@@ -256,7 +261,7 @@ ts_parse_byid(PG_FUNCTION_ARGS)
text *txt = PG_GETARG_TEXT_PP(1);
funcctx = SRF_FIRSTCALL_INIT();
- prs_setup_firstcall(funcctx, PG_GETARG_OID(0), txt);
+ prs_setup_firstcall(funcctx, fcinfo, PG_GETARG_OID(0), txt);
PG_FREE_IF_COPY(txt, 1);
}
@@ -281,7 +286,7 @@ ts_parse_byname(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
prsId = get_ts_parser_oid(textToQualifiedNameList(prsname), false);
- prs_setup_firstcall(funcctx, prsId, txt);
+ prs_setup_firstcall(funcctx, fcinfo, prsId, txt);
}
funcctx = SRF_PERCALL_SETUP();
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index b5b117a8ca..8afda0e5d2 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -4983,19 +4983,10 @@ pg_timezone_abbrevs(PG_FUNCTION_ARGS)
*pindex = 0;
funcctx->user_fctx = (void *) pindex;
- /*
- * build tupdesc for result tuples. This must match this function's
- * pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "abbrev",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "utc_offset",
- INTERVALOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "is_dst",
- BOOLOID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
+
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index d678d28600..7808fbd448 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -427,18 +427,9 @@ pg_get_keywords(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- tupdesc = CreateTemplateTupleDesc(5);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "word",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "catcode",
- CHAROID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "barelabel",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "catdesc",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "baredesc",
- TEXTOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
@@ -515,20 +506,8 @@ pg_get_catalog_foreign_keys(PG_FUNCTION_ARGS)
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
- tupdesc = CreateTemplateTupleDesc(6);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "fktable",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "fkcols",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "pktable",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "pkcols",
- TEXTARRAYOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "is_array",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "is_opt",
- BOOLOID, -1, 0);
-
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
funcctx->tuple_desc = BlessTupleDesc(tupdesc);
/*
diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c
index 96b5ae52d2..84518630a5 100644
--- a/src/backend/utils/adt/partitionfuncs.c
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -88,17 +88,9 @@ pg_partition_tree(PG_FUNCTION_ARGS)
*/
partitions = find_all_inheritors(rootrelid, AccessShareLock, NULL);
- tupdesc = CreateTemplateTupleDesc(PG_PARTITION_TREE_COLS);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
- REGCLASSOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "isleaf",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "level",
- INT4OID, -1, 0);
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
/* The only state we need is the partition list */
funcctx->user_fctx = (void *) partitions;
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index f7c1e3d6d6..caeb85b4ca 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -647,7 +647,9 @@ tsvector_unnest(PG_FUNCTION_ARGS)
INT2ARRAYOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "weights",
TEXTARRAYOID, -1, 0);
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->user_fctx = PG_GETARG_TSVECTOR_COPY(0);
@@ -2302,14 +2304,9 @@ ts_setup_firstcall(FunctionCallInfo fcinfo, FuncCallContext *funcctx,
}
Assert(stat->stackpos <= stat->maxdepth);
- tupdesc = CreateTemplateTupleDesc(3);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "word",
- TEXTOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "ndoc",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "nentry",
- INT4OID, -1, 0);
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ funcctx->tuple_desc = tupdesc;
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 781f8b8758..bbe3c6b365 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -217,22 +217,8 @@ pg_control_recovery(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(5);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "min_recovery_end_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "min_recovery_end_timeline",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "backup_start_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "backup_end_lsn",
- PG_LSNOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "end_of_backup_record_required",
- BOOLOID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* read the control file */
ControlFile = get_controlfile(DataDir, &crc_ok);
@@ -270,34 +256,8 @@ pg_control_init(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
bool crc_ok;
- /*
- * Construct a tuple descriptor for the result row. This must match this
- * function's pg_proc entry!
- */
- tupdesc = CreateTemplateTupleDesc(11);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "max_data_alignment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "database_block_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "blocks_per_segment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "wal_block_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 5, "bytes_per_wal_segment",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 6, "max_identifier_length",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 7, "max_index_columns",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 8, "max_toast_chunk_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 9, "large_object_chunk_size",
- INT4OID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 10, "float8_pass_by_value",
- BOOLOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 11, "data_page_checksum_version",
- INT4OID, -1, 0);
- tupdesc = BlessTupleDesc(tupdesc);
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
/* read the control file */
ControlFile = get_controlfile(DataDir, &crc_ok);
--
2.34.1
v2-0002-Remove-unnecessary-BlessTupleDesc-after-get_call_.patchapplication/x-patch; name=v2-0002-Remove-unnecessary-BlessTupleDesc-after-get_call_.patchDownload
From 5e51cfb41ab41cae9be319d36f26c4b5a1fc556e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 19 Dec 2022 13:02:59 +0000
Subject: [PATCH v2] Remove unnecessary BlessTupleDesc() after
get_call_result_type()
get_call_result_type() already does what BlessTupleDesc() intends
to do for the returned tuple descriptor. Hence, it is unnecessary
to call BlessTupleDesc() it right after get_call_result_type().
---
contrib/hstore/hstore_op.c | 6 +-----
contrib/pageinspect/brinfuncs.c | 1 -
contrib/pageinspect/btreefuncs.c | 2 --
contrib/pageinspect/hashfuncs.c | 2 --
contrib/sslinfo/sslinfo.c | 6 +-----
src/backend/statistics/mcv.c | 1 -
src/backend/utils/adt/misc.c | 2 +-
src/test/regress/regress.c | 1 -
8 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/contrib/hstore/hstore_op.c b/contrib/hstore/hstore_op.c
index 0d4ec16d1e..2b3a1e6a2f 100644
--- a/contrib/hstore/hstore_op.c
+++ b/contrib/hstore/hstore_op.c
@@ -862,13 +862,9 @@ setup_firstcall(FuncCallContext *funcctx, HStore *hs,
if (fcinfo)
{
- TupleDesc tupdesc;
-
/* Build a tuple descriptor for our result type */
- if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ if (get_call_result_type(fcinfo, NULL, &funcctx->tuple_desc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
-
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
}
MemoryContextSwitchTo(oldcontext);
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 12a7217038..b812cdeee3 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -348,7 +348,6 @@ brin_metapage_info(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupdesc = BlessTupleDesc(tupdesc);
/* Extract values from the metapage */
meta = (BrinMetaPageData *) PageGetContents(page);
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 9375d55e14..5e083a6f9c 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -537,7 +537,6 @@ bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
uargs->tupd = tupleDesc;
@@ -653,7 +652,6 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
uargs->tupd = tupleDesc;
diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c
index 81815392d7..e1364c5518 100644
--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -259,7 +259,6 @@ hash_page_stats(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
j = 0;
values[j++] = Int32GetDatum(stat.live_items);
@@ -334,7 +333,6 @@ hash_page_items(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupleDesc = BlessTupleDesc(tupleDesc);
fctx->attinmeta = TupleDescGetAttInMetadata(tupleDesc);
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index 5fd46b9874..6e313ced5e 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -370,9 +370,6 @@ ssl_extension_info(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
-
- TupleDesc tupdesc;
-
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
@@ -385,11 +382,10 @@ ssl_extension_info(PG_FUNCTION_ARGS)
fctx = (SSLExtensionInfoContext *) palloc(sizeof(SSLExtensionInfoContext));
/* Construct tuple descriptor */
- if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ if (get_call_result_type(fcinfo, NULL, &fctx->tupdesc) != TYPEFUNC_COMPOSITE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("function returning record called in context that cannot accept type record")));
- fctx->tupdesc = BlessTupleDesc(tupdesc);
/* Set max_calls as a count of extensions in certificate */
max_calls = cert != NULL ? X509_get_ext_count(cert) : 0;
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index f5a7c31272..592c76fea7 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1371,7 +1371,6 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("function returning record called in context "
"that cannot accept type record")));
- tupdesc = BlessTupleDesc(tupdesc);
/*
* generate attribute metadata needed later to produce tuples from raw
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 7808fbd448..a2a2df2adb 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -508,7 +508,7 @@ pg_get_catalog_foreign_keys(PG_FUNCTION_ARGS)
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+ funcctx->tuple_desc = tupdesc;
/*
* We use array_in to convert the C strings in sys_fk_relationships[]
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 2977045cc7..4d5e3e0918 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -1133,7 +1133,6 @@ test_enc_conversion(PG_FUNCTION_ARGS)
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
- tupdesc = BlessTupleDesc(tupdesc);
srclen = VARSIZE_ANY_EXHDR(string);
src = VARDATA_ANY(string);
--
2.34.1
On Tue, Dec 13, 2022 at 10:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Saving code is nice, but I'd assume the result is slower, because
get_call_result_type has to do a pretty substantial amount of work
to get the data to construct the tupdesc from. Have you tried to
quantify how much overhead this'd add? Which of these functions
can we safely consider to be non-performance-critical?
Here's a modest proposal: let's do nothing about this. There's no
evidence of a real problem here, so we're going to be trying to judge
the performance benefits against the code size savings without any
real data indicating that either one is an issue. I bet we could
convert all of these to one style or the other and it would make very
little real world difference, but deciding which ones to change and in
which direction will take up time and energy that could otherwise be
spent on more worthwhile projects, and could possibly complicate
back-patching, too.
Basically, I think this is nit-picking. Let's just accept that both
styles have some advantages and leave it up to patch authors to pick
one that they prefer.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 2022-Dec-19, Robert Haas wrote:
Here's a modest proposal: let's do nothing about this. There's no
evidence of a real problem here, so we're going to be trying to judge
the performance benefits against the code size savings without any
real data indicating that either one is an issue. I bet we could
convert all of these to one style or the other and it would make very
little real world difference, but deciding which ones to change and in
which direction will take up time and energy that could otherwise be
spent on more worthwhile projects, and could possibly complicate
back-patching, too.Basically, I think this is nit-picking. Let's just accept that both
styles have some advantages and leave it up to patch authors to pick
one that they prefer.
The code savings are substantial actually, so I think bloating things
for cases where performance is not an issue is not good. Some other
developer is sure to cargo-cult that stuff in the future, and that's not
great.
On the other hand, the measurements have shown that going through the
function is significantly slower. So I kinda like the judgement call
that Michael and Bharath have made: change to use the function when
performance is not an issue, and keep the verbose coding otherwise.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Mon, Dec 19, 2022 at 2:07 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On the other hand, the measurements have shown that going through the
function is significantly slower. So I kinda like the judgement call
that Michael and Bharath have made: change to use the function when
performance is not an issue, and keep the verbose coding otherwise.
Seems fairly arbitrary to me. The ones used for monitoring queries
aren't likely to be run often enough that it matters, but in theory
it's possible that they could be. Many of the ones supposedly not used
for monitoring queries could reasonably be so used, too. You can get
any answer you want by making arbitrary assumptions about which ones
are likely to be used frequently and how frequently they're likely to
be used, and I think different people evaluating the list
independently of each other and with no knowledge of each others work
would likely reach substantially different conclusions, ranging all
the way from "do them all this way" to "do them all the other way" and
various positions in the middle.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, Dec 19, 2022 at 2:07 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On the other hand, the measurements have shown that going through the
function is significantly slower. So I kinda like the judgement call
that Michael and Bharath have made: change to use the function when
performance is not an issue, and keep the verbose coding otherwise.
Seems fairly arbitrary to me.
Agreed ... but the decisions embodied in the code-as-it-stands are
even more arbitrary, being no doubt mostly based on "which function
did you copy to start from" not on any thought about performance.
Now that somebody's made an effort to identify which places are
potentially performance-critical, I don't see why we wouldn't use
the fruits of their labor. Yes, somebody else might draw the line
differently, but drawing a line at all seems like a step forward
to me.
regards, tom lane
On Mon, Dec 19, 2022 at 4:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Now that somebody's made an effort to identify which places are
potentially performance-critical, I don't see why we wouldn't use
the fruits of their labor. Yes, somebody else might draw the line
differently, but drawing a line at all seems like a step forward
to me.
All right, well, I just work here. :-)
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, Dec 19, 2022 at 05:50:03PM -0500, Robert Haas wrote:
All right, well, I just work here. :-)
Just to give some numbers. The original version of the patch doing
the full switch removed 500 lines of code. The second version that
switches the "non-critical" paths removes 200~ lines.
--
Michael
On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote:
I agree with the bucketization. Please see the attached patches. 0001
- gets rid of explicit tuple desc creation using
get_call_result_type() for functions thought to be not-so-frequently
called.
It looks like I am OK with the code paths updated here, which refer to
none of the "critical" function paths.
0002 - gets rid of an unnecessary call to BlessTupleDesc()
after get_call_result_type().
Hmm. I am not sure whether this is right, actually..
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote:
0002 - gets rid of an unnecessary call to BlessTupleDesc()
after get_call_result_type().
Hmm. I am not sure whether this is right, actually..
Hmm ... at least one of the paths through internal_get_result_type
is intentionally blessing the result tupdesc:
if (tupdesc->tdtypeid == RECORDOID &&
tupdesc->tdtypmod < 0)
assign_record_type_typmod(tupdesc);
but it's not clear if they all do, and the comments certainly
aren't promising it.
I'd be in favor of making this a documented API promise,
but it isn't that right now.
regards, tom lane
On Tue, Dec 20, 2022 at 1:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Michael Paquier <michael@paquier.xyz> writes:
On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote:
0002 - gets rid of an unnecessary call to BlessTupleDesc()
after get_call_result_type().Hmm. I am not sure whether this is right, actually..
Hmm ... at least one of the paths through internal_get_result_type
is intentionally blessing the result tupdesc:if (tupdesc->tdtypeid == RECORDOID &&
tupdesc->tdtypmod < 0)
assign_record_type_typmod(tupdesc);but it's not clear if they all do, and the comments certainly
aren't promising it.
It looks to be safe to get rid of BlessTupleDesc() after
get_call_result_type() for the functions that have OUT parameters and
return 'record' type. This is because, the
get_call_result_type()->internal_get_result_type()->build_function_result_tupdesc_t()
returns non-NULL tupdesc for such functions and all the functions that
0002 patch touches are having OUT parameters and their return type is
'record'. I've also verified with Assert(tupdesc->tdtypmod >= 0); -
https://github.com/BRupireddy/postgres/tree/test_for_tdypmod_init_v1.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
On Tue, Dec 20, 2022 at 1:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hmm ... at least one of the paths through internal_get_result_type
is intentionally blessing the result tupdesc:
but it's not clear if they all do, and the comments certainly
aren't promising it.
It looks to be safe to get rid of BlessTupleDesc() after
get_call_result_type() for the functions that have OUT parameters and
return 'record' type.
I think it's an absolutely horrid idea for callers to depend on
such details of get_call_result_type's behavior --- especially
when there is no function documentation promising it.
If we want to do something here, the thing to do would be to
guarantee in get_call_result_type's API spec that any returned
tupledesc is blessed. However, that might make some other
cases slower, if they don't need that.
On the whole, I'm content to leave the BlessTupleDesc calls in
these callers. They are cheap enough if the tupdesc is already
blessed.
regards, tom lane
On Tue, Dec 20, 2022 at 11:12:09AM -0500, Tom Lane wrote:
On the whole, I'm content to leave the BlessTupleDesc calls in
these callers. They are cheap enough if the tupdesc is already
blessed.
Yeah, agreed.
I have applied v2-0001, after fixing one error in wparser.c where some
of the previous style was not removed, leading to unnecessary work and
the same TupleDesc being built twice for the two ts_token_type()'s
(input of OID or text).
--
Michael
On Wed, Dec 21, 2022 at 6:44 AM Michael Paquier <michael@paquier.xyz> wrote:
I have applied v2-0001.
Thanks for taking care of this.
By seeing the impact that get_call_result_type() can have for the
functions that are possibly called repeatedly, I couldn't resist
sharing a patch (attached herewith) that adds a note of caution and
another way to build TupleDesc in the documentation to help developers
out there. Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Add-another-way-to-build-TupleDesc-in-documentati.patchapplication/x-patch; name=v1-0001-Add-another-way-to-build-TupleDesc-in-documentati.patchDownload
From 3ee0c4a402bcf2d25a7c544ddd60d3e9742b9048 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 21 Dec 2022 06:38:27 +0000
Subject: [PATCH v1] Add another way to build TupleDesc in documentation
---
doc/src/sgml/xfunc.sgml | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index cf5810b3c1..8b9c718fdf 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -2839,6 +2839,35 @@ TypeFuncClass get_call_result_type(FunctionCallInfo fcinfo,
cannot accept type record</quote>.)
</para>
+ <para>
+ Another way to setup <structname>TupleDesc</structname> is to use the
+ following functions:
+<programlisting>
+TupleDesc CreateTemplateTupleDesc(int natts);
+void TupleDescInitEntry(TupleDesc desc,
+ AttrNumber attributeNumber,
+ const char *attributeName,
+ Oid oidtypeid,
+ int32 typmod,
+ int attdim);
+TupleDesc BlessTupleDesc(TupleDesc tupdesc);
+</programlisting>
+ For example:
+<programlisting>
+ /*
+ * Construct a tuple descriptor for the result row. The number of output
+ * columns, column names and types must match the function's pg_proc entry
+ * or function's definition.
+ */
+ tupdesc = CreateTemplateTupleDesc(4);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, "foo", INT4OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 2, "bar", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 3, "baz", TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 4, "qux", TIMESTAMPTZOID, -1, 0);
+ tupdesc = BlessTupleDesc(tupdesc);
+</programlisting>
+ </para>
+
<tip>
<para>
<function>get_call_result_type</function> can resolve the actual type of a
@@ -2849,6 +2878,21 @@ TypeFuncClass get_call_result_type(FunctionCallInfo fcinfo,
</para>
</tip>
+ <note>
+ <para>
+ <function>get_call_result_type</function> is relatively expensive
+ as it has to look up system catalogs and do other things to setup
+ <structname>TupleDesc</structname>. While it is recommended
+ in most of the cases, remember to consider setting up
+ <structname>TupleDesc</structname> with
+ <function>CreateTemplateTupleDesc</function>,
+ <function>TupleDescInitEntry</function> and
+ <function>BlessTupleDesc</function> as shown above, if the function
+ returns set or the function is possible used repeatedly
+ (monitoring purposes, for instance).
+ </para>
+ </note>
+
<note>
<para>
<function>get_call_result_type</function> has a sibling
--
2.34.1