RE: Allow logical replication to copy tables in binary format
Takamichi Osumi (Fujitsu) <osumi.takamichi@fujitsu.com>
From: "Takamichi Osumi (Fujitsu)" <osumi.takamichi@fujitsu.com>
To: 'Melih Mutlu' <m.melihmutlu@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
Cc: Euler Taveira <euler@eulerto.com>, Amit Kapila <amit.kapila16@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-12T03:07:24Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Allow logical replication to copy tables in binary format.
- ecb696527c01 16.0 landed
-
Common function for percent placeholder replacement
- c96de2ce1782 16.0 cited
-
Doc: add XML ID attributes to <sectN> and <varlistentry> tags.
- 78ee60ed84bb 16.0 cited
-
Re-order disable_on_error in tab-complete.
- d547f7cf5efc 16.0 cited
-
Make subscription tests pass with log_error_verbosity=verbose
- 19408aae7fa2 15.0 cited
-
Weaken type-OID-matching checks in array_recv and record_recv.
- 670c0a1d474b 14.0 cited
-
Allow logical replication to transfer data in binary format.
- 9de77b545313 14.0 cited
On Wednesday, January 11, 2023 7:45 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
> Thanks for your review.
> Done.
Hi, minor comments on v5.
(1) publisher's version check
+ /* If the publisher is v16 or later, copy in binary format.*/
+ server_version = walrcv_server_version(LogRepWorkerWalRcvConn);
+ if (server_version >=160000 && MySubscription->binary)
+ {
+ appendStringInfoString(&cmd, " WITH (FORMAT binary)");
+ options = lappend(options, makeDefElem("format", (Node *) makeString("binary"), -1));
+ }
+
+ elog(LOG, "version: %i, %s", server_version, cmd.data);
(1-1) There is no need to log the version and the query by elog here.
(1-2) Also, I suggest we can remove the server_version variable itself,
because we have only one actual reference for it.
There is a style that we call walrcv_server_version in the
'if condition' directly like existing codes in fetch_remote_table_info().
(1-3) Suggestion to improve comments.
FROM:
/* If the publisher is v16 or later, copy in binary format.*/
TO:
/* If the publisher is v16 or later, copy data in the required data format. */
(2) Minor suggestion for some test code alignment.
$result =
$node_subscriber->safe_psql('postgres',
"SELECT sum(a) FROM tst_dom_constr");
-is($result, '21', 'sql-function constraint on domain');
+is($result, '33', 'sql-function constraint on domain');
+
+$result_binary =
+ $node_subscriber->safe_psql('postgres',
+ "SELECT sum(a) FROM tst_dom_constr");
+is($result_binary, '33', 'sql-function constraint on domain');
I think if we change the order of this part of check like below, then
it would look more aligned with other existing test codes introduced by this patch.
---
my $domain_check = 'SELECT sum(a) FROM tst_dom_constr';
$result = $node_subscriber->safe_psql('postgres', $domain_check);
$result_binary = $node_subscriber->safe_psql('postgres', $domain_check);
is($result, '33', 'sql-function constraint on domain');
is($result_binary, '33', 'sql-function constraint on domain in binary');
---
Best Regards,
Takamichi Osumi