On Thu, Dec 15, 2022 at 11:41 AM Michael Paquier <mich...@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 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. 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
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
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