public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 05/10] add special pg_brin_bloom_summary data type 9+ messages / 4 participants [nested] [flat]
* [PATCH 05/10] add special pg_brin_bloom_summary data type @ 2020-08-06 15:56 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 9+ messages in thread From: Tomas Vondra @ 2020-08-06 15:56 UTC (permalink / raw) --- src/backend/access/brin/brin_bloom.c | 91 ++++++++++++++++++++++- src/include/catalog/pg_proc.dat | 14 ++++ src/include/catalog/pg_type.dat | 7 ++ src/test/regress/expected/type_sanity.out | 7 +- 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index b119e4e264..e7ca100821 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -647,7 +647,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS) result->oi_regular_nulls = true; result->oi_opaque = (BloomOpaque *) MAXALIGN((char *) result + SizeofBrinOpcInfo(1)); - result->oi_typcache[0] = lookup_type_cache(BYTEAOID, 0); + result->oi_typcache[0] = lookup_type_cache(BRINBLOOMSUMMARYOID, 0); PG_RETURN_POINTER(result); } @@ -978,3 +978,92 @@ brin_bloom_options(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +/* + * brin_bloom_summary_in + * - input routine for type brin_bloom_summary. + * + * brin_bloom_summary is only used internally to represent summaries + * in BRIN bloom indexes, so it has no operations of its own, and we + * disallow input too. + */ +Datum +brin_bloom_summary_in(PG_FUNCTION_ARGS) +{ + /* + * brin_bloom_summary stores the data in binary form and parsing + * text input is not needed, so disallow this. + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot accept a value of type %s", "pg_brin_bloom_summary"))); + + PG_RETURN_VOID(); /* keep compiler quiet */ +} + + +/* + * brin_bloom_summary_out + * - output routine for type brin_bloom_summary. + * + * BRIN bloom summaries are serialized into a bytea value, but we want + * to output something nicer humans can understand. + */ +Datum +brin_bloom_summary_out(PG_FUNCTION_ARGS) +{ + BloomFilter *filter; + StringInfoData str; + + initStringInfo(&str); + appendStringInfoChar(&str, '{'); + + /* + * XXX not sure the detoasting is necessary (probably not, this + * can only be in an index). + */ + filter = (BloomFilter *) PG_DETOAST_DATUM(PG_GETARG_BYTEA_PP(0)); + + if (BLOOM_IS_HASHED(filter)) + { + appendStringInfo(&str, "mode: hashed nhashes: %u nbits: %u nbits_set: %u", + filter->nhashes, filter->nbits, filter->nbits_set); + } + else + { + appendStringInfo(&str, "mode: sorted nvalues: %u nsorted: %u", + filter->nvalues, filter->nsorted); + /* TODO include the sorted/unsorted values */ + } + + appendStringInfoChar(&str, '}'); + + PG_RETURN_CSTRING(str.data); +} + +/* + * brin_bloom_summary_recv + * - binary input routine for type brin_bloom_summary. + */ +Datum +brin_bloom_summary_recv(PG_FUNCTION_ARGS) +{ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot accept a value of type %s", "pg_brin_bloom_summary"))); + + PG_RETURN_VOID(); /* keep compiler quiet */ +} + +/* + * brin_bloom_summary_send + * - binary output routine for type brin_bloom_summary. + * + * BRIN bloom summaries are serialized in a bytea value (although the + * type is named differently), so let's just send that. + */ +Datum +brin_bloom_summary_send(PG_FUNCTION_ARGS) +{ + return byteasend(fcinfo); +} diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index c00e4ece94..dc17d4ce38 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10993,3 +10993,17 @@ prosrc => 'unicode_is_normalized' }, ] + +{ oid => '9035', descr => 'I/O', + proname => 'brin_bloom_summary_in', prorettype => 'pg_brin_bloom_summary', + proargtypes => 'cstring', prosrc => 'brin_bloom_summary_in' }, +{ oid => '9036', descr => 'I/O', + proname => 'brin_bloom_summary_out', prorettype => 'cstring', + proargtypes => 'pg_brin_bloom_summary', prosrc => 'brin_bloom_summary_out' }, +{ oid => '9037', descr => 'I/O', + proname => 'brin_bloom_summary_recv', provolatile => 's', + prorettype => 'pg_brin_bloom_summary', proargtypes => 'internal', + prosrc => 'brin_bloom_summary_recv' }, +{ oid => '9038', descr => 'I/O', + proname => 'brin_bloom_summary_send', provolatile => 's', prorettype => 'bytea', + proargtypes => 'pg_brin_bloom_summary', prosrc => 'brin_bloom_summary_send' }, diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat index b2cec07416..a41c2e5418 100644 --- a/src/include/catalog/pg_type.dat +++ b/src/include/catalog/pg_type.dat @@ -631,3 +631,10 @@ typalign => 'd', typstorage => 'x' }, ] + +{ oid => '9034', oid_symbol => 'BRINBLOOMSUMMARYOID', + descr => 'BRIN bloom summary', + typname => 'pg_brin_bloom_summary', typlen => '-1', typbyval => 'f', typcategory => 'S', + typinput => 'brin_bloom_summary_in', typoutput => 'brin_bloom_summary_out', + typreceive => 'brin_bloom_summary_recv', typsend => 'brin_bloom_summary_send', + typalign => 'i', typstorage => 'x', typcollation => 'default' }, diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out index 274130e706..97bf9797de 100644 --- a/src/test/regress/expected/type_sanity.out +++ b/src/test/regress/expected/type_sanity.out @@ -67,13 +67,14 @@ WHERE p1.typtype not in ('c','d','p') AND p1.typname NOT LIKE E'\\_%' (SELECT 1 FROM pg_type as p2 WHERE p2.typname = ('_' || p1.typname)::name AND p2.typelem = p1.oid and p1.typarray = p2.oid); - oid | typname -------+----------------- + oid | typname +------+----------------------- 194 | pg_node_tree 3361 | pg_ndistinct 3402 | pg_dependencies 5017 | pg_mcv_list -(4 rows) + 9034 | pg_brin_bloom_summary +(5 rows) -- Make sure typarray points to a varlena array type of our own base SELECT p1.oid, p1.typname as basetype, p2.typname as arraytype, -- 2.25.4 --6k3bcoookz7x4sns Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0006-BRIN-minmax-multi-indexes-20200911.patch" ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-23 22:40 Masahiko Sawada <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Masahiko Sawada @ 2024-10-23 22:40 UTC (permalink / raw) To: Peter Smith <[email protected]>; +Cc: Michael Paquier <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Oct 23, 2024 at 3:26 PM Peter Smith <[email protected]> wrote: > > On Thu, Oct 24, 2024 at 8:26 AM Masahiko Sawada <[email protected]> wrote: > > Thanks for your feedback! > > > > > While the changes look good to me, the comment of GetPublicationsStr() > > seems not match what the function actually does: > > > > +/* > > + * Add publication names from the list to a string. > > + */ > > +void > > +GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal) > > > > It's true that the function adds publication names to the given > > StringInfo, but it seems to me that the function expects the string is > > empty. For example, if we call this function twice with the same > > publication list, say 'pub1' and 'pub2', it would return a string > > 'pub1,pub2pub1,pub2'. > > No, although this function is not designed to be called twice in a > row, there are code examples where this function is being called > passing (non-empty) SQL cmd string. > > e.g. > cmd = makeStringInfo(); > appendStringInfoString(cmd, "SELECT t.pubname FROM\n" > " pg_catalog.pg_publication t WHERE\n" > " t.pubname IN ("); > GetPublicationsStr(publications, cmd, true); > appendStringInfoChar(cmd, ')'); Thanks, that makes sense. > > > I think we can improve the description of this > > function to something like "Build a comma-separated string from the > > given list of publication names.". > > > > This is a refactoring patch, so I hadn't intended to touch the > function, but I agree the function comment could be better. > > Now, I've changed the function comment to: > /* > * Add a comma-separated list of publication names to the 'dest' string. > */ Thank you for updating the patch. The patch looks good to me. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 04:17 Michael Paquier <[email protected]> parent: Masahiko Sawada <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Michael Paquier @ 2024-10-24 04:17 UTC (permalink / raw) To: Masahiko Sawada <[email protected]>; +Cc: Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Oct 23, 2024 at 03:40:19PM -0700, Masahiko Sawada wrote: > > Now, I've changed the function comment to: > > /* > > * Add a comma-separated list of publication names to the 'dest' string. > > */ > > Thank you for updating the patch. The patch looks good to me. + /* Build the pub_names comma-separated string. */ + GetPublicationsStr(MySubscription->publications, pub_names, true); In fetch_remote_table_info(), it is true that we may finish by building the same list two times, but your patch also changes the logic so as the string is built for nothing when dealing with a server version of 14 or older. That's a waste in these cases. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 06:27 Peter Smith <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Peter Smith @ 2024-10-24 06:27 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Oct 24, 2024 at 3:17 PM Michael Paquier <[email protected]> wrote: > > On Wed, Oct 23, 2024 at 03:40:19PM -0700, Masahiko Sawada wrote: > > > Now, I've changed the function comment to: > > > /* > > > * Add a comma-separated list of publication names to the 'dest' string. > > > */ > > > > Thank you for updating the patch. The patch looks good to me. > > + /* Build the pub_names comma-separated string. */ > + GetPublicationsStr(MySubscription->publications, pub_names, true); > > In fetch_remote_table_info(), it is true that we may finish by > building the same list two times, but your patch also changes the > logic so as the string is built for nothing when dealing with a server > version of 14 or older. That's a waste in these cases. > -- Yes, well spotted -- I was aware of that. Originally I had coded a >= PG15 check for that pub_names assignment. e.g. if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000) GetPublicationsStr(MySubscription->publications, pub_names, true); But, somehow it seemed messy to do that just to cater for something I thought was not particularly likely. OTOH, I am happy to put that check back in if you think it is warranted. ====== Kind RegArds, Peter Smith. Fujitsu Australia ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 06:41 Michael Paquier <[email protected]> parent: Peter Smith <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Michael Paquier @ 2024-10-24 06:41 UTC (permalink / raw) To: Peter Smith <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Oct 24, 2024 at 05:27:25PM +1100, Peter Smith wrote: > Yes, well spotted -- I was aware of that. Originally I had coded a >= > PG15 check for that pub_names assignment. e.g. > > if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000) > GetPublicationsStr(MySubscription->publications, pub_names, true); I was wondering about putting the call of GetPublicationsStr() in the first block with the makeStringInfo(), have an Assert checking that pub_names->data is never NULL in the second block, and destroy the StringInfo only if it has been allocated. > But, somehow it seemed messy to do that just to cater for something I > thought was not particularly likely. OTOH, I am happy to put that > check back in if you think it is warranted. I think I would add it. The publication list is not mandatory, but your patch makes it look so. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 06:44 Peter Smith <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Peter Smith @ 2024-10-24 06:44 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Oct 24, 2024 at 5:41 PM Michael Paquier <[email protected]> wrote: > > On Thu, Oct 24, 2024 at 05:27:25PM +1100, Peter Smith wrote: > > Yes, well spotted -- I was aware of that. Originally I had coded a >= > > PG15 check for that pub_names assignment. e.g. > > > > if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000) > > GetPublicationsStr(MySubscription->publications, pub_names, true); > > I was wondering about putting the call of GetPublicationsStr() in the > first block with the makeStringInfo(), have an Assert checking that > pub_names->data is never NULL in the second block, and destroy the > StringInfo only if it has been allocated. > > > But, somehow it seemed messy to do that just to cater for something I > > thought was not particularly likely. OTOH, I am happy to put that > > check back in if you think it is warranted. > > I think I would add it. The publication list is not mandatory, but > your patch makes it look so. > -- No problem. I will post an updated patch tomorrow. ====== Kind Regards, Peter Smith. Fujitsu Australia ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 22:28 Peter Smith <[email protected]> parent: Peter Smith <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Peter Smith @ 2024-10-24 22:28 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Oct 24, 2024 at 5:44 PM Peter Smith <[email protected]> wrote: > > On Thu, Oct 24, 2024 at 5:41 PM Michael Paquier <[email protected]> wrote: > > > > On Thu, Oct 24, 2024 at 05:27:25PM +1100, Peter Smith wrote: > > > Yes, well spotted -- I was aware of that. Originally I had coded a >= > > > PG15 check for that pub_names assignment. e.g. > > > > > > if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000) > > > GetPublicationsStr(MySubscription->publications, pub_names, true); > > > > I was wondering about putting the call of GetPublicationsStr() in the > > first block with the makeStringInfo(), have an Assert checking that > > pub_names->data is never NULL in the second block, and destroy the > > StringInfo only if it has been allocated. > > > > > But, somehow it seemed messy to do that just to cater for something I > > > thought was not particularly likely. OTOH, I am happy to put that > > > check back in if you think it is warranted. > > > > I think I would add it. The publication list is not mandatory, but > > your patch makes it look so. > > -- > > No problem. I will post an updated patch tomorrow. > I've attached the patch v4. ====== Kind Regards, Peter Smith. Fujitsu Australia Attachments: [application/octet-stream] v4-0001-Refactor-to-use-common-function-GetPublicationsSt.patch (10.1K, ../../CAHut+PtnPja2vpGHVjwAXkrJucWZqVzmifueNTuxXgjYLA0VOw@mail.gmail.com/2-v4-0001-Refactor-to-use-common-function-GetPublicationsSt.patch) download | inline diff: From 6ddf93eedfff98e591a6b8fc9b609337b05cf9e2 Mon Sep 17 00:00:00 2001 From: Peter Smith <[email protected]> Date: Fri, 25 Oct 2024 09:05:36 +1100 Subject: [PATCH v4] Refactor to use common function 'GetPublicationsStr'. There is already a utility function (called 'get_publications_str') for building a comma-separated string of publication names, but it was not being used everywhere it could have been. This is a refactoring patch to make better use of this existing function. Changes: - The function has been moved, and renamed to 'GetPublicationsStr', because it is no longer static. - Now GetPublicationsStr() is also being called from tablesync.c, fetch_remote_table_info(). - fetch_remote_table_info() was building the same publication string multiple times. In passing, fixed to build the string only once. - Similarly, there were duplicated calls subscriptioncmds.c fetch_table_list(). This was not causing a performance hit -- just more code than was needed. In passing, simplified this too. --- src/backend/catalog/pg_subscription.c | 31 +++++++++++++++ src/backend/commands/subscriptioncmds.c | 60 +++++++---------------------- src/backend/replication/logical/tablesync.c | 35 +++++------------ src/include/catalog/pg_subscription.h | 5 ++- 4 files changed, 58 insertions(+), 73 deletions(-) diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index 9efc915..89bf5ec 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -35,6 +35,37 @@ static List *textarray_to_stringlist(ArrayType *textarray); /* + * Add a comma-separated list of publication names to the 'dest' string. + */ +void +GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal) +{ + ListCell *lc; + bool first = true; + + Assert(publications != NIL); + + foreach(lc, publications) + { + char *pubname = strVal(lfirst(lc)); + + if (first) + first = false; + else + appendStringInfoString(dest, ", "); + + if (quote_literal) + appendStringInfoString(dest, quote_literal_cstr(pubname)); + else + { + appendStringInfoChar(dest, '"'); + appendStringInfoString(dest, pubname); + appendStringInfoChar(dest, '"'); + } + } +} + +/* * Fetch the subscription from the syscache. */ Subscription * diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 02ccc63..f7f6701 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -440,37 +440,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, } /* - * Add publication names from the list to a string. - */ -static void -get_publications_str(List *publications, StringInfo dest, bool quote_literal) -{ - ListCell *lc; - bool first = true; - - Assert(publications != NIL); - - foreach(lc, publications) - { - char *pubname = strVal(lfirst(lc)); - - if (first) - first = false; - else - appendStringInfoString(dest, ", "); - - if (quote_literal) - appendStringInfoString(dest, quote_literal_cstr(pubname)); - else - { - appendStringInfoChar(dest, '"'); - appendStringInfoString(dest, pubname); - appendStringInfoChar(dest, '"'); - } - } -} - -/* * Check that the specified publications are present on the publisher. */ static void @@ -486,7 +455,7 @@ check_publications(WalReceiverConn *wrconn, List *publications) appendStringInfoString(cmd, "SELECT t.pubname FROM\n" " pg_catalog.pg_publication t WHERE\n" " t.pubname IN ("); - get_publications_str(publications, cmd, true); + GetPublicationsStr(publications, cmd, true); appendStringInfoChar(cmd, ')'); res = walrcv_exec(wrconn, cmd->data, 1, tableRow); @@ -523,7 +492,7 @@ check_publications(WalReceiverConn *wrconn, List *publications) /* Prepare the list of non-existent publication(s) for error message. */ StringInfo pubnames = makeStringInfo(); - get_publications_str(publicationsCopy, pubnames, false); + GetPublicationsStr(publicationsCopy, pubnames, false); ereport(WARNING, errcode(ERRCODE_UNDEFINED_OBJECT), errmsg_plural("publication %s does not exist on the publisher", @@ -2151,7 +2120,7 @@ check_publications_origin(WalReceiverConn *wrconn, List *publications, " JOIN pg_subscription_rel PS ON (GPT.relid = PS.srrelid),\n" " pg_class C JOIN pg_namespace N ON (N.oid = C.relnamespace)\n" "WHERE C.oid = GPT.relid AND P.pubname IN ("); - get_publications_str(publications, &cmd, true); + GetPublicationsStr(publications, &cmd, true); appendStringInfoString(&cmd, ")\n"); /* @@ -2208,7 +2177,7 @@ check_publications_origin(WalReceiverConn *wrconn, List *publications, StringInfo pubnames = makeStringInfo(); /* Prepare the list of publication(s) for warning message. */ - get_publications_str(publist, pubnames, false); + GetPublicationsStr(publist, pubnames, false); ereport(WARNING, errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("subscription \"%s\" requested copy_data with origin = NONE but might copy data that had a different origin", @@ -2243,17 +2212,17 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications) List *tablelist = NIL; int server_version = walrcv_server_version(wrconn); bool check_columnlist = (server_version >= 150000); + StringInfo pub_names = makeStringInfo(); initStringInfo(&cmd); + /* Build the pub_names comma-separated string. */ + GetPublicationsStr(publications, pub_names, true); + /* Get the list of tables from the publisher. */ if (server_version >= 160000) { - StringInfoData pub_names; - tableRow[2] = INT2VECTOROID; - initStringInfo(&pub_names); - get_publications_str(publications, &pub_names, true); /* * From version 16, we allowed passing multiple publications to the @@ -2275,9 +2244,7 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications) " FROM pg_publication\n" " WHERE pubname IN ( %s )) AS gpt\n" " ON gpt.relid = c.oid\n", - pub_names.data); - - pfree(pub_names.data); + pub_names->data); } else { @@ -2288,12 +2255,13 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications) if (check_columnlist) appendStringInfoString(&cmd, ", t.attnames\n"); - appendStringInfoString(&cmd, "FROM pg_catalog.pg_publication_tables t\n" - " WHERE t.pubname IN ("); - get_publications_str(publications, &cmd, true); - appendStringInfoChar(&cmd, ')'); + appendStringInfo(&cmd, "FROM pg_catalog.pg_publication_tables t\n" + " WHERE t.pubname IN ( %s )", + pub_names->data); } + destroyStringInfo(pub_names); + res = walrcv_exec(wrconn, cmd.data, check_columnlist ? 3 : 2, tableRow); pfree(cmd.data); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index e03e761..e8bf736 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -802,7 +802,7 @@ fetch_remote_table_info(char *nspname, char *relname, Oid qualRow[] = {TEXTOID}; bool isnull; int natt; - ListCell *lc; + StringInfo pub_names = NULL; Bitmapset *included_cols = NULL; lrel->nspname = nspname; @@ -856,15 +856,10 @@ fetch_remote_table_info(char *nspname, char *relname, WalRcvExecResult *pubres; TupleTableSlot *tslot; Oid attrsRow[] = {INT2VECTOROID}; - StringInfoData pub_names; - initStringInfo(&pub_names); - foreach(lc, MySubscription->publications) - { - if (foreach_current_index(lc) > 0) - appendStringInfoString(&pub_names, ", "); - appendStringInfoString(&pub_names, quote_literal_cstr(strVal(lfirst(lc)))); - } + /* Build the pub_names comma-separated string. */ + pub_names = makeStringInfo(); + GetPublicationsStr(MySubscription->publications, pub_names, true); /* * Fetch info about column lists for the relation (from all the @@ -881,7 +876,7 @@ fetch_remote_table_info(char *nspname, char *relname, " WHERE gpt.relid = %u AND c.oid = gpt.relid" " AND p.pubname IN ( %s )", lrel->remoteid, - pub_names.data); + pub_names->data); pubres = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data, lengthof(attrsRow), attrsRow); @@ -936,8 +931,6 @@ fetch_remote_table_info(char *nspname, char *relname, ExecDropSingleTupleTableSlot(tslot); walrcv_clear_result(pubres); - - pfree(pub_names.data); } /* @@ -1039,19 +1032,8 @@ fetch_remote_table_info(char *nspname, char *relname, */ if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000) { - StringInfoData pub_names; - - /* Build the pubname list. */ - initStringInfo(&pub_names); - foreach_node(String, pubstr, MySubscription->publications) - { - char *pubname = strVal(pubstr); - - if (foreach_current_index(pubstr) > 0) - appendStringInfoString(&pub_names, ", "); - - appendStringInfoString(&pub_names, quote_literal_cstr(pubname)); - } + /* Reuse the already built pub_names comma-separated string. */ + Assert(pub_names); /* Check for row filters. */ resetStringInfo(&cmd); @@ -1062,7 +1044,7 @@ fetch_remote_table_info(char *nspname, char *relname, " WHERE gpt.relid = %u" " AND p.pubname IN ( %s )", lrel->remoteid, - pub_names.data); + pub_names->data); res = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data, 1, qualRow); @@ -1101,6 +1083,7 @@ fetch_remote_table_info(char *nspname, char *relname, ExecDropSingleTupleTableSlot(slot); walrcv_clear_result(res); + destroyStringInfo(pub_names); } pfree(cmd.data); diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h index 0aa14ec..b25f3fe 100644 --- a/src/include/catalog/pg_subscription.h +++ b/src/include/catalog/pg_subscription.h @@ -20,7 +20,7 @@ #include "access/xlogdefs.h" #include "catalog/genbki.h" #include "catalog/pg_subscription_d.h" - +#include "lib/stringinfo.h" #include "nodes/pg_list.h" /* @@ -180,4 +180,7 @@ extern void DisableSubscription(Oid subid); extern int CountDBSubscriptions(Oid dbid); +extern void GetPublicationsStr(List *publications, StringInfo dest, + bool quote_literal); + #endif /* PG_SUBSCRIPTION_H */ -- 1.8.3.1 ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-24 22:59 Michael Paquier <[email protected]> parent: Peter Smith <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Michael Paquier @ 2024-10-24 22:59 UTC (permalink / raw) To: Peter Smith <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Oct 25, 2024 at 09:28:47AM +1100, Peter Smith wrote: > I've attached the patch v4. Looks OK to me. Thanks. I'll see to get that done through the day. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Refactor to use common function 'get_publications_str'. @ 2024-10-25 05:03 Peter Smith <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 9+ messages in thread From: Peter Smith @ 2024-10-25 05:03 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Oct 25, 2024 at 10:00 AM Michael Paquier <[email protected]> wrote: > > On Fri, Oct 25, 2024 at 09:28:47AM +1100, Peter Smith wrote: > > I've attached the patch v4. > > Looks OK to me. Thanks. I'll see to get that done through the day. > -- Thanks for pushing! ====== Kind Regards, Peter Smith. Fujitsu Australia ^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2024-10-25 05:03 UTC | newest] Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-08-06 15:56 [PATCH 05/10] add special pg_brin_bloom_summary data type Tomas Vondra <[email protected]> 2024-10-23 22:40 Re: Refactor to use common function 'get_publications_str'. Masahiko Sawada <[email protected]> 2024-10-24 04:17 ` Re: Refactor to use common function 'get_publications_str'. Michael Paquier <[email protected]> 2024-10-24 06:27 ` Re: Refactor to use common function 'get_publications_str'. Peter Smith <[email protected]> 2024-10-24 06:41 ` Re: Refactor to use common function 'get_publications_str'. Michael Paquier <[email protected]> 2024-10-24 06:44 ` Re: Refactor to use common function 'get_publications_str'. Peter Smith <[email protected]> 2024-10-24 22:28 ` Re: Refactor to use common function 'get_publications_str'. Peter Smith <[email protected]> 2024-10-24 22:59 ` Re: Refactor to use common function 'get_publications_str'. Michael Paquier <[email protected]> 2024-10-25 05:03 ` Re: Refactor to use common function 'get_publications_str'. Peter Smith <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox