public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bharath Rupireddy <[email protected]>
To: Melih Mutlu <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Takamichi Osumi (Fujitsu) <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Allow logical replication to copy tables in binary format
Date: Tue, 21 Feb 2023 19:18:00 +0530
Message-ID: <CALj2ACVPt-BaLMm3Ezy1-rfUzH9qStxePcyGrHPamPESEZSBFA@mail.gmail.com> (raw)
In-Reply-To: <CAGPVpCTULGLwNGH7mMytR8dP1zg5f3wWq4=V=z=nT=-t8=2Vjg@mail.gmail.com>
References: <CAGPVpCQvAziCLknEnygY0v1-KBtg+Om-9JHJYZOnNPKFJPompw@mail.gmail.com>
	<[email protected]>
	<CAA4eK1KxYj=3rYHbPMQvNSFytHdyS5=q0D3A1T=uerC7HMFCtg@mail.gmail.com>
	<[email protected]>
	<CAGPVpCR15YfE=4hthY6S2OC5G0nj0QM4oE2bV769+FANM_UvwQ@mail.gmail.com>
	<[email protected]>
	<CAGPVpCRjdFHa71Qqd4Xj22UTa5=-NBUPgtT==X0w7TfWUZrpUg@mail.gmail.com>
	<TYCPR01MB837354C136F914D2AE174AE1ED489@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CAGPVpCR_g8c1xXfR4kkSXiSt314C0it0seRzfqgTd+oF7N_5jw@mail.gmail.com>
	<TYCPR01MB8373B593010467315C2BA8EBED229@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CAGPVpCRaxRA-SqWYsWvqWPB_CfVyasvCwC2oqg9oQgZfP0QBtQ@mail.gmail.com>
	<OSZPR01MB63105595F660457538F9024EFDFC9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<CAGPVpCR2ffpiNYea-cU4sXaijCV4GHiUBf=yupKbVFM0=zWtVw@mail.gmail.com>
	<TYCPR01MB83731E3ACC68CCEBD2E48541EDFD9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CAGPVpCS0OzZ1tx3qT1Lc83HVK2JCc=TQ1G73tbyHYDKyo+mzFQ@mail.gmail.com>
	<CALj2ACU47pbGUUxyxVbGg3RAb8JsZhUuK+KogPyiWxYZbGVrxg@mail.gmail.com>
	<CAGPVpCSS_S8ZHiV+=3RvsDro16gJhVckzQDUrZ08b3LSPWTmsA@mail.gmail.com>
	<CALj2ACW5Oa7_v25iZb326UXvtM_tjQfw0Tc3hPJ8zN4FZqc9cw@mail.gmail.com>
	<CAGPVpCQYi9AYQSS=RmGgVNjz5ZEnLB8mACwd9aioVhLmbgiAMA@mail.gmail.com>
	<TYAPR01MB5866F36CF5D12DFEE90B0B8BF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CAGPVpCTULGLwNGH7mMytR8dP1zg5f3wWq4=V=z=nT=-t8=2Vjg@mail.gmail.com>

On Mon, Feb 20, 2023 at 5:17 PM Melih Mutlu <[email protected]> wrote:
>
> Thanks for letting me know.
> Attached the fixed version of the patch.

Thanks. I have few comments on v9 patch:

1.
+                    /* Do not allow binary = false with copy_format = binary */
+                    if (!opts.binary &&
+                        sub->copyformat == LOGICALREP_COPY_AS_BINARY &&
+                        !IsSet(opts.specified_opts, SUBOPT_COPY_FORMAT))
+                        ereport(ERROR,
+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                 errmsg("cannot set %s for a
subscription with %s",
+                                        "binary = false",
"copy_format = binary")));

I don't understand why we'd need to tie an option (binary) that deals
with data types at column-level with another option (copy_format) that
requests the entire table data to be in binary. This'd essentially
make one to set binary = true to use copy_format = binary, no? IMHO,
this inter-dependency is not good for better usability.

2. Why can't the tests that this patch adds be simple? Why would it
need to change the existing tests at all? I'm thinking to create a new
00X_binary_copy_format.pl or such and setting up logical replication
with copy_format = binary and letting table sync worker request
publisher in binary format - you can verify this via publisher server
logs - look for COPY with BINARY option. If required, have the table
with different data types. This greatly reduces the patch's footprint.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






view thread (14+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Allow logical replication to copy tables in binary format
  In-Reply-To: <CALj2ACVPt-BaLMm3Ezy1-rfUzH9qStxePcyGrHPamPESEZSBFA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox