On Fri, Mar 25, 2022 at 3:29 AM Tomas Vondra
<tomas.von...@enterprisedb.com> wrote:
>
> Hi,
>
> Pushed, after going through the patch once more, addressed the remaining
> FIXMEs, corrected a couple places in the docs and comments, etc. Minor
> tweaks, nothing important.

While rebasing patch [1] I found a couple of comments:
static void
 ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
-    List **rels, List **schemas)
+    List **tables, List **sequences,
+    List **tables_schemas, List **sequences_schemas,
+    List **schemas)
 {
  ListCell   *cell;
  PublicationObjSpec *pubobj;
@@ -185,12 +194,23 @@ ObjectsInPublicationToOids(List
*pubobjspec_list, ParseState *pstate,
  switch (pubobj->pubobjtype)
  {
  case PUBLICATIONOBJ_TABLE:
- *rels = lappend(*rels, pubobj->pubtable);
+ *tables = lappend(*tables, pubobj->pubtable);
+ break;
+ case PUBLICATIONOBJ_SEQUENCE:
+ *sequences = lappend(*sequences, pubobj->pubtable);
  break;
  case PUBLICATIONOBJ_TABLES_IN_SCHEMA:
  schemaid = get_namespace_oid(pubobj->name, false);

  /* Filter out duplicates if user specifies "sch1, sch1" */
+ *tables_schemas = list_append_unique_oid(*tables_schemas, schemaid);
+ *schemas = list_append_unique_oid(*schemas, schemaid);
+ break;

Now tables_schemas and sequence_schemas are being updated and used in
ObjectsInPublicationToOids, schema parameter is no longer being used
after processing in ObjectsInPublicationToOids, I felt we can remove
that parameter.

  /* ALTER PUBLICATION <name> ADD */
  else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD"))
- COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE");
+ COMPLETE_WITH("ALL TABLES IN SCHEMA", "ALL SEQUENCES IN SCHEMA",
"TABLE", "SEQUENCE");

Tab completion of alter publication for ADD and DROP is the same, we
could combine it.

Attached a patch for the same.
Thoughts?

[1] - 
https://www.postgresql.org/message-id/CALDaNm3%3DJrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh%3DtamA%40mail.gmail.com

Regards,
Vignesh
From dc707ba93494e3ba0cffd8ab6e1fb1b7a1c0e70e Mon Sep 17 00:00:00 2001
From: Vigneshwaran C <vignes...@gmail.com>
Date: Fri, 25 Mar 2022 19:49:40 +0530
Subject: [PATCH] Removed unused parameter from ObjectsInPublicationToOids.

Removed unused parameter from ObjectsInPublicationToOids.
---
 src/backend/commands/publicationcmds.c | 15 +++------------
 src/bin/psql/tab-complete.c            |  5 +----
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index c6437799c5..89298d7857 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -175,8 +175,7 @@ parse_publication_options(ParseState *pstate,
 static void
 ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
 						   List **tables, List **sequences,
-						   List **tables_schemas, List **sequences_schemas,
-						   List **schemas)
+						   List **tables_schemas, List **sequences_schemas)
 {
 	ListCell   *cell;
 	PublicationObjSpec *pubobj;
@@ -204,14 +203,12 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
 
 				/* Filter out duplicates if user specifies "sch1, sch1" */
 				*tables_schemas = list_append_unique_oid(*tables_schemas, schemaid);
-				*schemas = list_append_unique_oid(*schemas, schemaid);
 				break;
 			case PUBLICATIONOBJ_SEQUENCES_IN_SCHEMA:
 				schemaid = get_namespace_oid(pubobj->name, false);
 
 				/* Filter out duplicates if user specifies "sch1, sch1" */
 				*sequences_schemas = list_append_unique_oid(*sequences_schemas, schemaid);
-				*schemas = list_append_unique_oid(*schemas, schemaid);
 				break;
 			case PUBLICATIONOBJ_TABLES_IN_CUR_SCHEMA:
 				search_path = fetch_search_path(false);
@@ -225,7 +222,6 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
 
 				/* Filter out duplicates if user specifies "sch1, sch1" */
 				*tables_schemas = list_append_unique_oid(*tables_schemas, schemaid);
-				*schemas = list_append_unique_oid(*schemas, schemaid);
 				break;
 			case PUBLICATIONOBJ_SEQUENCES_IN_CUR_SCHEMA:
 				search_path = fetch_search_path(false);
@@ -239,7 +235,6 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
 
 				/* Filter out duplicates if user specifies "sch1, sch1" */
 				*sequences_schemas = list_append_unique_oid(*sequences_schemas, schemaid);
-				*schemas = list_append_unique_oid(*schemas, schemaid);
 				break;
 			default:
 				/* shouldn't happen */
@@ -679,7 +674,6 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 	List	   *sequences = NIL;
 	List	   *tables_schemaidlist = NIL;
 	List	   *sequences_schemaidlist = NIL;
-	List	   *schemaidlist = NIL;
 
 	bool		for_all_tables = false;
 	bool		for_all_sequences = false;
@@ -782,8 +776,7 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 		ObjectsInPublicationToOids(stmt->pubobjects, pstate,
 								   &tables, &sequences,
 								   &tables_schemaidlist,
-								   &sequences_schemaidlist,
-								   &schemaidlist);
+								   &sequences_schemaidlist);
 
 		/* FOR ALL TABLES IN SCHEMA requires superuser */
 		if (list_length(tables_schemaidlist) > 0 && !superuser())
@@ -1462,14 +1455,12 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
 		List	   *sequences = NIL;
 		List	   *tables_schemaidlist = NIL;
 		List	   *sequences_schemaidlist = NIL;
-		List	   *schemaidlist = NIL;
 		Oid			pubid = pubform->oid;
 
 		ObjectsInPublicationToOids(stmt->pubobjects, pstate,
 								   &tables, &sequences,
 								   &tables_schemaidlist,
-								   &sequences_schemaidlist,
-								   &schemaidlist);
+								   &sequences_schemaidlist);
 
 		CheckAlterPublication(stmt, tup,
 							  tables, tables_schemaidlist,
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 63bfdf11c6..682d4fe18d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1814,7 +1814,7 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches("ALTER", "PUBLICATION", MatchAny))
 		COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME TO", "SET");
 	/* ALTER PUBLICATION <name> ADD */
-	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD"))
+	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP"))
 		COMPLETE_WITH("ALL TABLES IN SCHEMA", "ALL SEQUENCES IN SCHEMA", "TABLE", "SEQUENCE");
 	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") ||
 			 (HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") &&
@@ -1840,9 +1840,6 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH(",", "WHERE (");
 	else if (HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE"))
 		COMPLETE_WITH(",");
-	/* ALTER PUBLICATION <name> DROP */
-	else if (Matches("ALTER", "PUBLICATION", MatchAny, "DROP"))
-		COMPLETE_WITH("ALL TABLES IN SCHEMA", "ALL SEQUENCES IN SCHEMA", "TABLE", "SEQUENCE");
 	/* ALTER PUBLICATION <name> SET */
 	else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET"))
 		COMPLETE_WITH("(", "ALL TABLES IN SCHEMA", "ALL SEQUENCES IN SCHEMA", "TABLE", "SEQUENCE");
-- 
2.32.0

Reply via email to