Re: Allow logical replication to copy tables in binary format

Hi,

Please see the attached v16.

Peter Smith <smithpb2250(at)gmail(dot)com>, 16 Mar 2023 Per, 03:03 tarihinde şunu
yazdı:

> Here are some review comments for v15-0001
>

I applied your comments in the updated patch.

shiy(dot)fnst(at)fujitsu(dot)com <shiy(dot)fnst(at)fujitsu(dot)com>, 16 Mar 2023 Per, 05:35
tarihinde şunu yazdı:

> On Thu, Mar 16, 2023 2:26 AM Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> wrote:
> >
> > Right, it needs to be ordered. Fixed.
> >
>
> Hi,
>
> Thanks for updating the patch. I tested some cases like toast data,
> combination
> of row filter and column lists, and it works well.
>

Thanks for testing. I changed wait_for_log lines as you suggested.

houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com>, 16 Mar 2023 Per, 05:55
tarihinde şunu yazdı:

> 1.
> + {
> + appendStringInfo(&cmd, " WITH (FORMAT binary)");
>
> We could use appendStringInfoString here.
>

Done.

> 2.
> I think it would be better to pass the log offset when using wait_for_log,
> because otherwise it will check the whole log file to find the target
> message,
> This might not be a big problem, but it has a risk of getting unexpected
> log message
> which was generated by previous commands.
>

You're right. I added offsets for wait_for_log's .

Thanks,
--
Melih Mutlu
Microsoft