Here are some review comments for patch v20240720-0002.
======
1. Commit message:
1a.
The commit message is stale. It is still referring to functions and
views that have been moved to patch 0003.
1b.
"ALL SEQUENCES" is not a command. It is a clause of the CREATE
PUBLICATION command.
======
doc/src/sgml/ref/create_publication.sgml
nitpick - publication name in the example /allsequences/all_sequences/
======
src/bin/psql/describe.c
2. describeOneTableDetails
Although it's not the fault of this patch, this patch propagates the
confusion of 'result' versus 'res'. Basically, I did not understand
the need for the variable 'result'. There is already a "PGResult
*res", and unless I am mistaken we can just keep re-using that instead
of introducing a 2nd variable having almost the same name and purpose.
~
nitpick - comment case
nitpick - rearrange comment
======
src/test/regress/expected/publication.out
(see publication.sql)
======
src/test/regress/sql/publication.sql
nitpick - tweak comment
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/doc/src/sgml/ref/create_publication.sgml
b/doc/src/sgml/ref/create_publication.sgml
index c9c1b92..7dcfe37 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -422,7 +422,7 @@ CREATE PUBLICATION users_filtered FOR TABLE users (user_id,
firstname);
<para>
Create a publication that synchronizes all the sequences:
<programlisting>
-CREATE PUBLICATION allsequences FOR ALL SEQUENCES;
+CREATE PUBLICATION all_sequences FOR ALL SEQUENCES;
</programlisting>
</para>
</refsect1>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0f3f86b..a92af54 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1851,7 +1851,7 @@ describeOneTableDetails(const char *schemaname,
}
PQclear(result);
- /* print any publications */
+ /* Print any publications */
if (pset.sversion >= 180000)
{
int tuples = 0;
@@ -1867,8 +1867,8 @@ describeOneTableDetails(const char *schemaname,
if (!result)
goto error_return;
- tuples = PQntuples(result);
/* Might be an empty set - that's ok */
+ tuples = PQntuples(result);
if (tuples > 0)
{
printTableAddFooter(&cont, _("Publications:"));
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index 3ea2224..6c573a1 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -259,7 +259,7 @@ Publications:
SET client_min_messages = 'ERROR';
CREATE PUBLICATION regress_pub_forallsequences2 FOR ALL SEQUENCES;
RESET client_min_messages;
--- Check describe sequence lists both the publications
+-- check that describe sequence lists all publications the sequence belongs to
\d+ pub_test.regress_pub_seq1
Sequence "pub_test.regress_pub_seq1"
Type | Start | Minimum | Maximum | Increment | Cycles? | Cache
diff --git a/src/test/regress/sql/publication.sql
b/src/test/regress/sql/publication.sql
index 8d553ed..ac77fe4 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -134,7 +134,7 @@ SET client_min_messages = 'ERROR';
CREATE PUBLICATION regress_pub_forallsequences2 FOR ALL SEQUENCES;
RESET client_min_messages;
--- Check describe sequence lists both the publications
+-- check that describe sequence lists all publications the sequence belongs to
\d+ pub_test.regress_pub_seq1
--- FOR ALL specifying both TABLES and SEQUENCES
