Skip to content

Commit

Permalink
Remove ALL keyword from TABLES IN SCHEMA for publication
Browse files Browse the repository at this point in the history
This may be a bit too subtle, but removing that word from there makes
this clause no longer a perfect parallel of the GRANT variant "ALL
TABLES IN SCHEMA": indeed, for publications what we record is the schema
itself, not the tables therein, which means that any tables added to the
schema in the future are also published.  This is completely different
to what GRANT does, which is affect only the tables that exist when the
command is executed.

There isn't resounding support for this change, but there are a few
positive votes and no opposition.  Because the time to 15 RC1 is very
short, let's get this out now.

Backpatch to 15.

Discussion: https://postgr.es/m/2729c9e2-9aac-8cda-f2f4-34f2bcc18f4e
  • Loading branch information
alvherre committed Sep 22, 2022
1 parent ba50834 commit 790bf61
Show file tree
Hide file tree
Showing 21 changed files with 176 additions and 179 deletions.
4 changes: 2 additions & 2 deletions doc/src/sgml/logical-replication.sgml
Expand Up @@ -700,7 +700,7 @@ test_sub=# SELECT * FROM t3;
<listitem>
<para>
one of the publications was created using
<literal>FOR ALL TABLES IN SCHEMA</literal> and the table belongs to
<literal>FOR TABLES IN SCHEMA</literal> and the table belongs to
the referred schema. This clause does not allow row filters.
</para>
</listitem>
Expand Down Expand Up @@ -1530,7 +1530,7 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
Moreover, if untrusted users can create tables, use only
publications that list tables explicitly. That is to say, create a
subscription <literal>FOR ALL TABLES</literal> or
<literal>FOR ALL TABLES IN SCHEMA</literal> only when superusers trust
<literal>FOR TABLES IN SCHEMA</literal> only when superusers trust
every user permitted to create a non-temp table on the publisher or the
subscriber.
</para>
Expand Down
16 changes: 8 additions & 8 deletions doc/src/sgml/ref/alter_publication.sgml
Expand Up @@ -31,7 +31,7 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r
<phrase>where <replaceable class="parameter">publication_object</replaceable> is one of:</phrase>

TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [ ( <replaceable class="parameter">column_name</replaceable> [, ... ] ) ] [ WHERE ( <replaceable class="parameter">expression</replaceable> ) ] [, ... ]
ALL TABLES IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
TABLES IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
</synopsis>
</refsynopsisdiv>

Expand Down Expand Up @@ -71,19 +71,19 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r
<para>
You must own the publication to use <command>ALTER PUBLICATION</command>.
Adding a table to a publication additionally requires owning that table.
The <literal>ADD ALL TABLES IN SCHEMA</literal> and
<literal>SET ALL TABLES IN SCHEMA</literal> to a publication requires the
The <literal>ADD TABLES IN SCHEMA</literal> and
<literal>SET TABLES IN SCHEMA</literal> to a publication requires the
invoking user to be a superuser. To alter the owner, you must also be a
direct or indirect member of the new owning role. The new owner must have
<literal>CREATE</literal> privilege on the database. Also, the new owner
of a <literal>FOR ALL TABLES</literal> or <literal>FOR ALL TABLES IN
SCHEMA</literal> publication must be a superuser. However, a superuser can
of a <literal>FOR ALL TABLES</literal> or <literal>FOR TABLES IN SCHEMA</literal>
publication must be a superuser. However, a superuser can
change the ownership of a publication regardless of these restrictions.
</para>

<para>
Adding/Setting a table that is part of schema specified in
<literal>ALL TABLES IN SCHEMA</literal>, adding/setting a schema to a
<literal>TABLES IN SCHEMA</literal>, adding/setting a schema to a
publication that already has a table that is part of the specified schema or
adding/setting a table to a publication that already has a table's schema as
part of the specified schema is not supported.
Expand Down Expand Up @@ -200,7 +200,7 @@ ALTER PUBLICATION mypublication SET TABLE users (user_id, firstname, lastname),
<structname>sales</structname> to the publication
<structname>sales_publication</structname>:
<programlisting>
ALTER PUBLICATION sales_publication ADD ALL TABLES IN SCHEMA marketing, sales;
ALTER PUBLICATION sales_publication ADD TABLES IN SCHEMA marketing, sales;
</programlisting>
</para>

Expand All @@ -210,7 +210,7 @@ ALTER PUBLICATION sales_publication ADD ALL TABLES IN SCHEMA marketing, sales;
<structname>production</structname> to the publication
<structname>production_publication</structname>:
<programlisting>
ALTER PUBLICATION production_publication ADD TABLE users, departments, ALL TABLES IN SCHEMA production;
ALTER PUBLICATION production_publication ADD TABLE users, departments, TABLES IN SCHEMA production;
</programlisting></para>
</refsect1>

Expand Down
14 changes: 7 additions & 7 deletions doc/src/sgml/ref/create_publication.sgml
Expand Up @@ -29,7 +29,7 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
<phrase>where <replaceable class="parameter">publication_object</replaceable> is one of:</phrase>

TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [ ( <replaceable class="parameter">column_name</replaceable> [, ... ] ) ] [ WHERE ( <replaceable class="parameter">expression</replaceable> ) ] [, ... ]
ALL TABLES IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
TABLES IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
</synopsis>
</refsynopsisdiv>

Expand Down Expand Up @@ -112,7 +112,7 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>

<para>
Specifying a table that is part of a schema specified by
<literal>FOR ALL TABLES IN SCHEMA</literal> is not supported.
<literal>FOR TABLES IN SCHEMA</literal> is not supported.
</para>
</listitem>
</varlistentry>
Expand All @@ -128,7 +128,7 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
</varlistentry>

<varlistentry>
<term><literal>FOR ALL TABLES IN SCHEMA</literal></term>
<term><literal>FOR TABLES IN SCHEMA</literal></term>
<listitem>
<para>
Marks the publication as one that replicates changes for all tables in
Expand Down Expand Up @@ -224,7 +224,7 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>

<para>
If <literal>FOR TABLE</literal>, <literal>FOR ALL TABLES</literal> or
<literal>FOR ALL TABLES IN SCHEMA</literal> are not specified, then the
<literal>FOR TABLES IN SCHEMA</literal> are not specified, then the
publication starts out with an empty set of tables. That is useful if
tables or schemas are to be added later.
</para>
Expand All @@ -243,7 +243,7 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
<para>
To add a table to a publication, the invoking user must have ownership
rights on the table. The <command>FOR ALL TABLES</command> and
<command>FOR ALL TABLES IN SCHEMA</command> clauses require the invoking
<command>FOR TABLES IN SCHEMA</command> clauses require the invoking
user to be a superuser.
</para>

Expand Down Expand Up @@ -354,7 +354,7 @@ CREATE PUBLICATION insert_only FOR TABLE mydata
all changes for all the tables present in the schema
<structname>production</structname>:
<programlisting>
CREATE PUBLICATION production_publication FOR TABLE users, departments, ALL TABLES IN SCHEMA production;
CREATE PUBLICATION production_publication FOR TABLE users, departments, TABLES IN SCHEMA production;
</programlisting>
</para>

Expand All @@ -363,7 +363,7 @@ CREATE PUBLICATION production_publication FOR TABLE users, departments, ALL TABL
the schemas <structname>marketing</structname> and
<structname>sales</structname>:
<programlisting>
CREATE PUBLICATION sales_publication FOR ALL TABLES IN SCHEMA marketing, sales;
CREATE PUBLICATION sales_publication FOR TABLES IN SCHEMA marketing, sales;
</programlisting></para>

<para>
Expand Down
2 changes: 1 addition & 1 deletion doc/src/sgml/ref/create_subscription.sgml
Expand Up @@ -372,7 +372,7 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
publications has no <literal>WHERE</literal> clause (referring to that
publish operation) or the publication is declared as
<literal>FOR ALL TABLES</literal> or
<literal>FOR ALL TABLES IN SCHEMA</literal>, rows are always published
<literal>FOR TABLES IN SCHEMA</literal>, rows are always published
regardless of the definition of the other expressions.
If the subscriber is a <productname>PostgreSQL</productname> version before
15 then any row filtering is ignored during the initial data synchronization
Expand Down
2 changes: 1 addition & 1 deletion doc/src/sgml/system-views.sgml
Expand Up @@ -2090,7 +2090,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
tables they contain. Unlike the underlying catalog
<link linkend="catalog-pg-publication-rel"><structname>pg_publication_rel</structname></link>,
this view expands publications defined as <literal>FOR ALL TABLES</literal>
and <literal>FOR ALL TABLES IN SCHEMA</literal>, so for such publications
and <literal>FOR TABLES IN SCHEMA</literal>, so for such publications
there will be a row for each eligible table.
</para>

Expand Down
4 changes: 2 additions & 2 deletions src/backend/catalog/pg_publication.c
Expand Up @@ -837,7 +837,7 @@ GetAllTablesPublicationRelations(bool pubviaroot)
/*
* Gets the list of schema oids for a publication.
*
* This should only be used FOR ALL TABLES IN SCHEMA publications.
* This should only be used FOR TABLES IN SCHEMA publications.
*/
List *
GetPublicationSchemas(Oid pubid)
Expand Down Expand Up @@ -957,7 +957,7 @@ GetSchemaPublicationRelations(Oid schemaid, PublicationPartOpt pub_partopt)
}

/*
* Gets the list of all relations published by FOR ALL TABLES IN SCHEMA
* Gets the list of all relations published by FOR TABLES IN SCHEMA
* publication.
*/
List *
Expand Down
6 changes: 3 additions & 3 deletions src/backend/commands/publicationcmds.c
Expand Up @@ -847,11 +847,11 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
ObjectsInPublicationToOids(stmt->pubobjects, pstate, &relations,
&schemaidlist);

/* FOR ALL TABLES IN SCHEMA requires superuser */
/* FOR TABLES IN SCHEMA requires superuser */
if (schemaidlist != NIL && !superuser())
ereport(ERROR,
errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to create FOR ALL TABLES IN SCHEMA publication"));
errmsg("must be superuser to create FOR TABLES IN SCHEMA publication"));

if (relations != NIL)
{
Expand Down Expand Up @@ -1979,7 +1979,7 @@ AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to change owner of publication \"%s\"",
NameStr(form->pubname)),
errhint("The owner of a FOR ALL TABLES IN SCHEMA publication must be a superuser.")));
errhint("The owner of a FOR TABLES IN SCHEMA publication must be a superuser.")));
}

form->pubowner = newOwnerId;
Expand Down
18 changes: 9 additions & 9 deletions src/backend/parser/gram.y
Expand Up @@ -10340,7 +10340,7 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec
* pub_obj is one of:
*
* TABLE table [, ...]
* ALL TABLES IN SCHEMA schema [, ...]
* TABLES IN SCHEMA schema [, ...]
*
*****************************************************************************/

Expand Down Expand Up @@ -10375,7 +10375,7 @@ CreatePublicationStmt:
;

/*
* FOR TABLE and FOR ALL TABLES IN SCHEMA specifications
* FOR TABLE and FOR TABLES IN SCHEMA specifications
*
* This rule parses publication objects with and without keyword prefixes.
*
Expand All @@ -10397,18 +10397,18 @@ PublicationObjSpec:
$$->pubtable->columns = $3;
$$->pubtable->whereClause = $4;
}
| ALL TABLES IN_P SCHEMA ColId
| TABLES IN_P SCHEMA ColId
{
$$ = makeNode(PublicationObjSpec);
$$->pubobjtype = PUBLICATIONOBJ_TABLES_IN_SCHEMA;
$$->name = $5;
$$->location = @5;
$$->name = $4;
$$->location = @4;
}
| ALL TABLES IN_P SCHEMA CURRENT_SCHEMA
| TABLES IN_P SCHEMA CURRENT_SCHEMA
{
$$ = makeNode(PublicationObjSpec);
$$->pubobjtype = PUBLICATIONOBJ_TABLES_IN_CUR_SCHEMA;
$$->location = @5;
$$->location = @4;
}
| ColId opt_column_list OptWhereClause
{
Expand Down Expand Up @@ -10484,7 +10484,7 @@ pub_obj_list: PublicationObjSpec
* pub_obj is one of:
*
* TABLE table_name [, ...]
* ALL TABLES IN SCHEMA schema_name [, ...]
* TABLES IN SCHEMA schema_name [, ...]
*
*****************************************************************************/

Expand Down Expand Up @@ -18424,7 +18424,7 @@ preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner)
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
errmsg("invalid publication object list"),
errdetail("One of TABLE or ALL TABLES IN SCHEMA must be specified before a standalone table or schema name."),
errdetail("One of TABLE or TABLES IN SCHEMA must be specified before a standalone table or schema name."),
parser_errposition(pubobj->location));

foreach(cell, pubobjspec_list)
Expand Down
3 changes: 1 addition & 2 deletions src/backend/replication/pgoutput/pgoutput.c
Expand Up @@ -1014,8 +1014,7 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
* need to check all the given publication-table mappings and report an
* error if any publications have a different column list.
*
* FOR ALL TABLES and FOR ALL TABLES IN SCHEMA implies "don't use column
* list".
* FOR ALL TABLES and FOR TABLES IN SCHEMA imply "don't use column list".
*/
foreach(lc, publications)
{
Expand Down
2 changes: 1 addition & 1 deletion src/bin/pg_dump/pg_dump.c
Expand Up @@ -4317,7 +4317,7 @@ dumpPublicationNamespace(Archive *fout, const PublicationSchemaInfo *pubsinfo)
query = createPQExpBuffer();

appendPQExpBuffer(query, "ALTER PUBLICATION %s ", fmtId(pubinfo->dobj.name));
appendPQExpBuffer(query, "ADD ALL TABLES IN SCHEMA %s;\n", fmtId(schemainfo->dobj.name));
appendPQExpBuffer(query, "ADD TABLES IN SCHEMA %s;\n", fmtId(schemainfo->dobj.name));

/*
* There is no point in creating drop query as the drop is done by schema
Expand Down
15 changes: 7 additions & 8 deletions src/bin/pg_dump/t/002_pg_dump.pl
Expand Up @@ -2544,23 +2544,22 @@
unlike => { exclude_dump_test_schema => 1, },
},
'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test' => {
'ALTER PUBLICATION pub3 ADD TABLES IN SCHEMA dump_test' => {
create_order => 51,
create_sql =>
'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test;',
'ALTER PUBLICATION pub3 ADD TABLES IN SCHEMA dump_test;',
regexp => qr/^
\QALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test;\E
\QALTER PUBLICATION pub3 ADD TABLES IN SCHEMA dump_test;\E
/xm,
like => { %full_runs, section_post_data => 1, },
unlike => { exclude_dump_test_schema => 1, },
},
'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA public' => {
'ALTER PUBLICATION pub3 ADD TABLES IN SCHEMA public' => {
create_order => 52,
create_sql =>
'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA public;',
regexp => qr/^
\QALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA public;\E
create_sql => 'ALTER PUBLICATION pub3 ADD TABLES IN SCHEMA public;',
regexp => qr/^
\QALTER PUBLICATION pub3 ADD TABLES IN SCHEMA public;\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
Expand Down
15 changes: 7 additions & 8 deletions src/bin/psql/tab-complete.c
Expand Up @@ -1820,7 +1820,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME TO", "SET");
/* ALTER PUBLICATION <name> ADD */
else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD"))
COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE");
COMPLETE_WITH("TABLES IN SCHEMA", "TABLE");
else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") ||
(HeadMatches("ALTER", "PUBLICATION", MatchAny, "ADD|SET", "TABLE") &&
ends_with(prev_wd, ',')))
Expand All @@ -1844,10 +1844,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH(",");
/* ALTER PUBLICATION <name> DROP */
else if (Matches("ALTER", "PUBLICATION", MatchAny, "DROP"))
COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE");
COMPLETE_WITH("TABLES IN SCHEMA", "TABLE");
/* ALTER PUBLICATION <name> SET */
else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET"))
COMPLETE_WITH("(", "ALL TABLES IN SCHEMA", "TABLE");
COMPLETE_WITH("(", "TABLES IN SCHEMA", "TABLE");
else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "ALL", "TABLES", "IN", "SCHEMA"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
" AND nspname NOT LIKE E'pg\\\\_%%'",
Expand Down Expand Up @@ -2990,9 +2990,9 @@ psql_completion(const char *text, int start, int end)

/* CREATE PUBLICATION */
else if (Matches("CREATE", "PUBLICATION", MatchAny))
COMPLETE_WITH("FOR TABLE", "FOR ALL TABLES", "FOR ALL TABLES IN SCHEMA", "WITH (");
COMPLETE_WITH("FOR TABLE", "FOR ALL TABLES", "FOR TABLES IN SCHEMA", "WITH (");
else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR"))
COMPLETE_WITH("TABLE", "ALL TABLES", "ALL TABLES IN SCHEMA");
COMPLETE_WITH("TABLE", "ALL TABLES", "TABLES IN SCHEMA");
else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL"))
COMPLETE_WITH("TABLES", "TABLES IN SCHEMA");
else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES"))
Expand All @@ -3015,8 +3015,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH(" WITH (");

/*
* Complete "CREATE PUBLICATION <name> FOR ALL TABLES IN SCHEMA <schema>,
* ..."
* Complete "CREATE PUBLICATION <name> FOR TABLES IN SCHEMA <schema>, ..."
*/
else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES", "IN", "SCHEMA"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
Expand Down Expand Up @@ -3836,7 +3835,7 @@ psql_completion(const char *text, int start, int end)
"ALL PROCEDURES IN SCHEMA",
"ALL ROUTINES IN SCHEMA",
"ALL SEQUENCES IN SCHEMA",
"ALL TABLES IN SCHEMA",
"TABLES IN SCHEMA",
"DATABASE",
"DOMAIN",
"FOREIGN DATA WRAPPER",
Expand Down
2 changes: 1 addition & 1 deletion src/test/regress/expected/alter_table.out
Expand Up @@ -4593,7 +4593,7 @@ create schema alter1;
create schema alter2;
create table alter1.t1 (a int);
set client_min_messages = 'ERROR';
create publication pub1 for table alter1.t1, all tables in schema alter2;
create publication pub1 for table alter1.t1, tables in schema alter2;
reset client_min_messages;
alter table alter1.t1 set schema alter2; -- should fail
ERROR: cannot move table "t1" to schema "alter2"
Expand Down
2 changes: 1 addition & 1 deletion src/test/regress/expected/object_address.out
Expand Up @@ -45,7 +45,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
-- suppress warning that depends on wal_level
SET client_min_messages = 'ERROR';
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
CREATE PUBLICATION addr_pub_schema FOR ALL TABLES IN SCHEMA addr_nsp;
CREATE PUBLICATION addr_pub_schema FOR TABLES IN SCHEMA addr_nsp;
RESET client_min_messages;
CREATE SUBSCRIPTION regress_addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
Expand Down

0 comments on commit 790bf61

Please sign in to comment.