Re: Synchronizing slots from primary to standby

Hi,

On Wed, Feb 28, 2024 at 12:29:01PM +0530, shveta malik wrote:
> On Wed, Feb 28, 2024 at 8:49 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> >
> > Few comments:
>
> Thanks for the feedback.
>
> > ===============
> > 1.
> > - if (logical)
> > + if (logical || !replication)
> > {
> >
> > Can we add a comment about connection types that require
> > ALWAYS_SECURE_SEARCH_PATH_SQL?
> >
> > 2.
> > Can we add a test case to demonstrate that the '=' operator can be
> > hijacked to do different things when the slotsync worker didn't use
> > ALWAYS_SECURE_SEARCH_PATH_SQL?
> >
>
> Here is the patch with new test added and improved comments.

Thanks!

A few comments:

1 ===

+ * used to run normal SQL queries

s/run normal SQL/run SQL/ ?

As mentioned up-thread I don't like that much the idea of creating such a test
but if we do then here are my comments:

2 ===

+CREATE FUNCTION myschema.myintne(bigint, int)

Should we explain why 'bigint, int' is important here (instead of
'int, int')?

3 ===

+# stage of syncing newly created slots. If the worker was not prepared
+# to handle such attacks, it would have failed during

Worth to mention the underlying check / function that would get an "unexpected"
result?

Except for the above (nit) comments the patch looks good to me.

Regards,

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