[Code Cleanup] : Small code cleanup in twophase.sql
Hi,
PFA small code cleanup in twophase.sql. Which contains a drop table
statement for 'test_prepared_savepoint'. Which, to me, appears to be
missing in the cleanup section of that file.
To support it I have below points:-
1) Grepping this table 'test_prepared_savepoint' shows occurrences
only in twophase.out & twophase.sql files. This means that table is
local to that sql test file and not used in any other test file.
2) I don't see any comment on why this was not added in the cleanup
section of twophase.sql, but drop for other two test tables are done.
3) I ran "make check-world" with the patch and I don't see any failures.
Kindly correct, if I missed anything.
Regards,
Nishant (EDB).
Attachments:
v1-0001-Small-code-cleanup-in-twophase.sql.patchapplication/octet-stream; name=v1-0001-Small-code-cleanup-in-twophase.sql.patchDownload
From 4af31a650c1a51f306c650f2fa6b67577ec39a96 Mon Sep 17 00:00:00 2001
From: Nishant Sharma <nishant.sharma@enterprisedb.com>
Date: Tue, 26 Sep 2023 18:15:28 +0530
Subject: [PATCH v1] Small code cleanup in twophase.sql
---
contrib/test_decoding/expected/twophase.out | 1 +
contrib/test_decoding/sql/twophase.sql | 1 +
2 files changed, 2 insertions(+)
diff --git a/contrib/test_decoding/expected/twophase.out b/contrib/test_decoding/expected/twophase.out
index e89dc74..517f20b 100644
--- a/contrib/test_decoding/expected/twophase.out
+++ b/contrib/test_decoding/expected/twophase.out
@@ -209,6 +209,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
-- cleanup and make sure results are also empty
DROP TABLE test_prepared1;
DROP TABLE test_prepared2;
+DROP TABLE test_prepared_savepoint;
-- show results. There should be nothing to show
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
data
diff --git a/contrib/test_decoding/sql/twophase.sql b/contrib/test_decoding/sql/twophase.sql
index aff5114..0244795 100644
--- a/contrib/test_decoding/sql/twophase.sql
+++ b/contrib/test_decoding/sql/twophase.sql
@@ -108,6 +108,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
-- cleanup and make sure results are also empty
DROP TABLE test_prepared1;
DROP TABLE test_prepared2;
+DROP TABLE test_prepared_savepoint;
-- show results. There should be nothing to show
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
--
1.8.3.1
Hi,
Any taker or rejector for above? -- It's a very small 'good to have' change
patch for cleanup.
Thanks,
Nishant (EDB).
On Tue, Sep 26, 2023 at 6:31 PM Nishant Sharma <
nishant.sharma@enterprisedb.com> wrote:
Show quoted text
Hi,
PFA small code cleanup in twophase.sql. Which contains a drop table
statement for 'test_prepared_savepoint'. Which, to me, appears to be
missing in the cleanup section of that file.To support it I have below points:-
1) Grepping this table 'test_prepared_savepoint' shows occurrences
only in twophase.out & twophase.sql files. This means that table is
local to that sql test file and not used in any other test file.2) I don't see any comment on why this was not added in the cleanup
section of twophase.sql, but drop for other two test tables are done.3) I ran "make check-world" with the patch and I don't see any failures.
Kindly correct, if I missed anything.
Regards,
Nishant (EDB).
On Tue, Sep 26, 2023 at 06:31:42PM +0530, Nishant Sharma wrote:
To support it I have below points:-
1) Grepping this table 'test_prepared_savepoint' shows occurrences
only in twophase.out & twophase.sql files. This means that table is
local to that sql test file and not used in any other test file.2) I don't see any comment on why this was not added in the cleanup
section of twophase.sql, but drop for other two test tables are done.3) I ran "make check-world" with the patch and I don't see any failures.
Note that sometimes tables are left around in the regression tests for
pg_upgrade, so as we can test dedicated upgrade paths for some object
types. But, here, I think you're right: this is not a table that
matters for an upgrade and the end of the test file also expects a
cleanup. So okay for me to drop this table here.
--
Michael