;.
The attached patch makes those changes. What do you think?
--
With Regards,
Amit Kapila.
fix_doc_create_sub_1.patch
Description: Binary data
.confl_active_logicalslot has
been updated
+# we now expect 3 conflicts reported as the counter persist across reloads
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ "select (confl_active_logicalslot = 4) from
pg_stat_database_conflicts where datname = 'testdb'", 't'),
+ 'confl_active_logicalslot updated') or die "Timed out waiting
confl_active_logicalslot to be updated";
The comment incorrectly mentions 3 conflicts whereas the query expects 4.
--
With Regards,
Amit Kapila.
On Fri, Apr 7, 2023 at 8:43 AM Andres Freund wrote:
>
> On 2023-04-06 12:10:57 +0530, Amit Kapila wrote:
> > Also, it seems you have removed the checks related to
> > slots, is it because PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT is only
> > used for logical slots? If so, do y
On Fri, Apr 7, 2023 at 6:55 AM Andres Freund wrote:
>
> On 2023-04-06 12:10:57 +0530, Amit Kapila wrote:
> > After this, I think for backends that have active slots, it would
> > simply cancel the current query. Will that be sufficient? Because we
> > want the backend
On Thu, Apr 6, 2023 at 6:32 PM Drouvot, Bertrand
wrote:
>
> Hi,
>
> On 4/6/23 11:55 AM, Amit Kapila wrote:
> > On Thu, Apr 6, 2023 at 12:10 PM Amit Kapila wrote:
> >>
> >> On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
> >> wrote:
> >>&
On Thu, Apr 6, 2023 at 11:29 AM Amit Kapila wrote:
>
> >
>
> This doesn't seem to be addressed in the latest version. And today, I
> think I see one more point about this doc change:
> +
> + A logical replication slot can also be created on a hot standby.
>
On Thu, Apr 6, 2023 at 12:10 PM Amit Kapila wrote:
>
> On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
> wrote:
> >
>
> Another comment on 0001.
> extern void CheckSlotRequirements(void);
> extern void CheckSlotPermissions(void);
> +extern void ResolveRecove
On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
wrote:
>
> On 4/5/23 3:15 PM, Amit Kapila wrote:
> > On Wed, Apr 5, 2023 at 6:14 PM Drouvot, Bertrand
> > wrote:
> >>
> >> On 4/5/23 12:28 PM, Amit Kapila wrote:
> >>> On Wed, Apr 5, 2023 at 2:41 PM
On Wed, Apr 5, 2023 at 6:14 PM Drouvot, Bertrand
wrote:
>
> On 4/5/23 12:28 PM, Amit Kapila wrote:
> > On Wed, Apr 5, 2023 at 2:41 PM Drouvot, Bertrand
> > wrote:
> >> Maybe we could change the doc with something among those lines instead?
> >>
> >>
On Wed, Apr 5, 2023 at 6:14 PM Drouvot, Bertrand
wrote:
>
> On 4/5/23 12:28 PM, Amit Kapila wrote:
> > On Wed, Apr 5, 2023 at 2:41 PM Drouvot, Bertrand
> > wrote:
>
> > minor nitpick:
> > +
> > + /* Intentional fall through to session cancel */
> &
On Wed, Apr 5, 2023 at 3:58 PM Amit Kapila wrote:
>
> On Wed, Apr 5, 2023 at 2:41 PM Drouvot, Bertrand
> wrote:
>
> minor nitpick:
> +
> + /* Intentional fall through to session cancel */
> + /* FALLTHROUGH */
>
> Do we need to repeat fall through twice in differe
On Wed, Apr 5, 2023 at 2:41 PM Drouvot, Bertrand
wrote:
>
> On 4/5/23 8:59 AM, Amit Kapila wrote:
> > On Mon, Apr 3, 2023 at 12:05 PM Amit Kapila wrote:
>
> > On further thinking, as such this shouldn't be a problem because all
> > the WAL records before P
On Mon, Apr 3, 2023 at 12:05 PM Amit Kapila wrote:
>
> On Mon, Apr 3, 2023 at 4:39 AM Alvaro Herrera wrote:
> >
> > > From 56a9559555918a99c202a0924f7b2ede9de4e75d Mon Sep 17 00:00:00 2001
> > > From: bdrouvotAWS
> > > Date: Tue, 7 Feb 2023 08:59:47 +00
On Tue, Apr 4, 2023 at 6:05 PM Drouvot, Bertrand
wrote:
>
> On 4/4/23 1:43 PM, Amit Kapila wrote:
> > On Tue, Apr 4, 2023 at 3:14 PM Drouvot, Bertrand
> > wrote:
> >>
> >
> >
> > +static inline bool
> > +LogicalReplicationSlo
d functions, you
are referring to and or invalidating it.
--
With Regards,
Amit Kapila.
l' doesn't seem to convey its actual
meaning because one can imagine that it indicates whether the slot is
logical which I don't think is the actual intent. One idea to simplify
this is to introduce a single function CanInvalidateSlot() or
something like that and move the logic from both the functions
SlotIsInvalid() and SlotIsNotConflicting() into the new function.
--
With Regards,
Amit Kapila.
p
How did you decide on the starting year as 2022?
--
With Regards,
Amit Kapila.
s
run_as_owner. So, +1 to use the new option name as run_as_owner.
--
With Regards,
Amit Kapila.
_CHANGE record. Then later using that state it gives an
error from the appropriate place in the CommitTs module. If my
understanding is correct then that appears to be a better design than
what the patch is currently doing. Also, the error message used in
error_commit_ts_disabled() seems to be better than the current one.
--
With Regards,
Amit Kapila.
myself discussed the same approach few
emails above. BTW, if we have this selective logic to wake
physical/logical walsenders and for standby's, we only wake logical
walsenders at the time of ApplyWalRecord() then do we need the new
conditional variable enhancement being discussed, and if so, why?
--
With Regards,
Amit Kapila.
rds is to avoid the situation
where it may not need to wait again after decoding very few records.
But probably the logic in WalSndWaitForWal() will help us to exit
before starting to wait by checking the replay location.
--
With Regards,
Amit Kapila.
to pgsql-hackers? As this is a
development project so better to keep the discussion on pgsql-hackers.
[1] -
https://www.postgresql.org/message-id/CAA4eK1%2B%2BY7a2SQq55DXT6neghZgj3j%2BpQ74%3D8zfT3k8Tkdj0ZA%40mail.gmail.com
[2] -
https://www.postgresql.org/message-id/CAA4eK1KZqvJsTt7OkS8AkxOKVvSpkQkPwsqzNmo10mFaVAKeZg%40mail.gmail.com
[3] -
https://www.postgresql.org/message-id/OS0PR01MB571646874A3E165D93999A9D94889%40OS0PR01MB5716.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
On Fri, Mar 31, 2023 at 7:14 PM Drouvot, Bertrand
wrote:
>
> On 3/31/23 1:58 PM, Amit Kapila wrote:
> > On Fri, Mar 31, 2023 at 4:17 PM Drouvot, Bertrand
> > wrote:
> >>
> >
> > + * This is needed for logical decoding on standby. Indeed the "problem&qu
Record() where the 0005 patch does
conditionvariable broadcast? Now, there doesn't seem to be anything
that distinguishes between logical and physical walsender but I guess
we can add a variable in WalSnd structure to identify it.
--
With Regards,
Amit Kapila.
On Thu, Mar 30, 2023 at 3:48 PM Amit Kapila wrote:
>
> On Thu, Mar 30, 2023 at 9:22 AM Peter Smith wrote:
> >
> > I have marked the CF entry for this patch as "ready for committer"
> >
>
> LGTM. I'll push this tomorrow unless there are more commen
On Wed, Mar 29, 2023 at 10:23 PM Zheng Li wrote:
>
> On Wed, Mar 29, 2023 at 5:13 AM Amit Kapila wrote:
> >
> > On Wed, Mar 29, 2023 at 2:49 AM Zheng Li wrote:
> > >
> > >
> > > I agree that a full fledged DDL deparser and DDL replication is too
>
gt; > >
> > > On Tuesday, March 28, 2023 12:13 PM houzj.f...@fujitsu.com
> > > wrote:
> > > >
> > > > On Monday, March 27, 2023 8:08 PM Amit Kapila
> > >
> > > > wrote:
> > > >
On Thu, Mar 30, 2023 at 9:22 AM Peter Smith wrote:
>
> I have marked the CF entry for this patch as "ready for committer"
>
LGTM. I'll push this tomorrow unless there are more comments for it.
--
With Regards,
Amit Kapila.
why not do it?
>
> You can't really use the fact they were not there before as a reason
> to not add them now -- There were no Asserts in the original code
> because this same logic was duplicated multiple times and was always
> within obvious scope of a particular switch (actio
On Wed, Mar 29, 2023 at 7:58 PM Tomas Vondra
wrote:
>
> On 3/29/23 11:51, Amit Kapila wrote:
> >>
> >> It's not clear to me what should be the exact behavior?
> >>
> >> I mean, imagine we're opening a connection for logical replication, and
On Tue, Mar 28, 2023 at 8:30 PM Masahiko Sawada wrote:
>
> On Tue, Mar 28, 2023 at 6:47 PM Amit Kapila wrote:
> >
> > > >
> > > > > > I think we can have same issues as you mentioned New table t1 is
> > > > > > added
>
609) exited with
>exit code 1
>
> I don't see why sequences should do anything else.
>
Is this behavior of TRUNCATE known or discussed previously? I can't
see any mention of this in the docs or commit message. I guess if we
want to follow such behavior it should be well documented so that it
won't be a surprise for users. I think we would face such cases in the
future as well. One of the similar cases we are discussing for DDL
replication where a higher version publisher could send some DDL
syntax that lower version subscribers won't support and will lead to
an error [1].
[1] -
https://www.postgresql.org/message-id/OS0PR01MB5716088E497BDCBCED7FC3DA94849%40OS0PR01MB5716.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
we have a clear answer to that then I think we will be in a
better position to answer your question.
[1] -
https://www.postgresql.org/message-id/OS0PR01MB5716088E497BDCBCED7FC3DA94849%40OS0PR01MB5716.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
; LGTM, by inspection. Thanks!
>
Pushed.
--
With Regards,
Amit Kapila.
On Wed, Mar 29, 2023 at 6:31 AM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Amit-san,
>
> > There is no need to break the link line. See attached.
>
> I understood your saying. I think it's better.
>
Pushed.
--
With Regards,
Amit Kapila.
[2] for the >= PG16 case.
>
> Or vice versa, if you prefer.
>
The current coding pattern looks neat to me.
--
With Regards,
Amit Kapila.
usly I preferred not to add a new line inside the tag, but it
> caused
> long-line. So I adjusted them not to be too short/long length.
>
There is no need to break the link line. See attached.
--
With Regards,
Amit Kapila.
v7-0001-Add-XML-ID-attributes-to-create_subscription.sgml.patch
Description: Binary data
On Mon, Mar 27, 2023 at 8:17 AM Masahiko Sawada wrote:
>
> On Fri, Mar 24, 2023 at 11:51 PM Kumar, Sachin wrote:
> >
> > > From: Amit Kapila
> > > > I think we won't be able to use same snapshot because the transaction
> > > > will
> >
an be used with the disable_on_error
Isn't it better to move the link-related part to the next line
wherever possible? Currently, it looks bit odd.
Why 0002 patch is part of this thread? I thought here we want to add
'ids' to entries corresponding to Create Subscription as we have adde
On Mon, Mar 27, 2023 at 5:37 PM Amit Kapila wrote:
>
> On Mon, Mar 27, 2023 at 12:07 PM Amit Kapila wrote:
> >
> > On Mon, Mar 27, 2023 at 2:52 AM Tom Lane wrote:
> > >
> >
> > > I suggest taking a couple of steps back from the minutiae of the
> >
On Mon, Mar 27, 2023 at 12:07 PM Amit Kapila wrote:
>
> On Mon, Mar 27, 2023 at 2:52 AM Tom Lane wrote:
> >
>
> > I suggest taking a couple of steps back from the minutiae of the
> > patch, and spending some hard effort thinking about how the thing
> > would
'll try to summarize the discussion we have till now on this
and share my thoughts on the same in a separate email.
Thanks for paying attention to this work!
[1] -
https://www.postgresql.org/message-id/OS0PR01MB571684CBF660D05B63B4412C94AB9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
[2] -
https://www.postgresql.org/message-id/CA%2BTgmoauXRQ3yDZNGTzXv_m1kdUnH1Ww%2BhwKmKUSjtyBh0Em2Q%40mail.gmail.com
--
With Regards,
Amit Kapila.
e of INSERTS for the newly added table/s
> also. IMO it is not only better for test completeness, but it causes
> readers to question why there are INSERTS for every other table except
> these ones.
>
The purpose of the test is to test the initial sync's interaction with
'publish_via_partition_root' option. So, adding Inserts after that for
replication doesn't serve any purpose and it also consumes test cycles
without any additional benefit. So, -1 for extending it further.
--
With Regards,
Amit Kapila.
On Fri, Mar 24, 2023 at 2:36 PM wangw.f...@fujitsu.com
wrote:
>
> On Fri, Mar 24, 2023 at 14:17 PM Amit Kapila wrote:
>
> And I found there is a problem in the three back-branch patches
> (HEAD_v21_0002*,
> REL15_* and REL14_*):
> In the function fetch_table_list, we use p
t; tap_pub_3, tap_pub_4a, tap_pub_4b, tap_pub_5a, tap_pub_5b,
> > > tap_pub_toast, tap_pub_inherits, tap_pub_viaroot_2, tap_pub_viaroot_1"
> >
> > I think this is the scenario we fixed : Simultaneously subscribing to two
> > publications that publish the parent and child respectively, then want to
> > use
> > the parent's identity and schema).
> >
>
> Yeah, but currently BOTH the tap_pub_viaroot_2, tap_pub_viaroot_1 are
> using "WITH (publish_via_partition_root)", so IMO the user would
> surely expect that only the root table would be published even when a
> subscription combines those publications. OTOH, I thought a subtle
> point of this patch is that now the same result will happen even if
> only ONE of the publications was using "WITH
> (publish_via_partition_root)". So it’s that scenario of “only ONE
> publication is using the option” that I thought ought to be explicitly
> tested.
>
The current change to existing tests is difficult to understand. I
suggest writing a separate test for row filter and then cover the
scenario you have suggested.
--
With Regards,
Amit Kapila.
On Thu, Mar 23, 2023 at 9:24 PM Kumar, Sachin wrote:
>
> > From: Amit Kapila
> > IIUC, this is possible only if tablesync process uses a snapshot different
> > than the
> > snapshot we have used to perform the initial schema sync, otherwise, this
> > shou
gress faster (in case
> > you/Amit considers this one better).
> >
>
> Thanks for updating the patches.
> The v2 patch LGTM.
>
Pushed.
--
With Regards,
Amit Kapila.
On Thu, Mar 23, 2023 at 2:48 AM Euler Taveira wrote:
>
> On Tue, Mar 21, 2023, at 8:18 AM, Amit Kapila wrote:
>
> Now, how do we avoid these problems even if we have our own version of
> functionality similar to pg_dump for selected objects? I guess we will
> face similar probl
state
>
> > SUBREL_STATE_DATASYNC). Which might be a big window for big databases.
>
>
>
>
>
> Refresh publication :-
>
> In refresh publication, subscriber does create a new replication slot hence ,
> we can’t run
>
> pg_dump with a snapshot which starts from origin(maybe this is not an issue
> at all). In this case
>
> it makes more sense for tableSync worker to do schema sync.
>
Can you please explain this problem with some examples?
--
With Regards,
Amit Kapila.
t; accidentally missing for a long time. Does anybody know for sure if
> the omission of this function from the documentation is deliberate? If
> nobody here knows, then maybe this can be asked/addressed in a
> separate thread.
>
It is better that you start a separate thread to discuss this question
as it is not related to this patch.
--
With Regards,
Amit Kapila.
> > other options as well on this page?
>
> I have analyzed same points and made patch that could be applied atop
> v19-0001.
> Please check 0002 patch.
>
Pushed the 0001. It may be better to start a separate thread for 0002.
--
With Regards,
Amit Kapila.
re references. Shall we add id to
other options as well on this page?
--
With Regards,
Amit Kapila.
However, if Alvaro and others think
that the current style is better then we should go ahead and do it
that way. I hope that we should be able to decide on this and get it
into PG16. Anyone else would like to weigh in here?
--
With Regards,
Amit Kapila.
On Wed, Mar 22, 2023 at 8:29 AM Masahiko Sawada wrote:
>
> On Tue, Mar 21, 2023 at 8:18 PM Amit Kapila wrote:
> >
> > On Tue, Mar 21, 2023 at 7:32 AM Euler Taveira wrote:
> >
> > > You should
> > > exclude them removing these objects from the TOC before
olumns. (b) the newer version could have new features (represented by
say new columns in existing catalogs or new catalogs) that the older
version of pg_dump has no knowledge of and will fail to get that data
and hence an inconsistent dump. The subscriber will easily be not in
sync due to that.
N
On Tue, Mar 21, 2023 at 2:16 PM Melih Mutlu wrote:
>
> Amit Kapila , 21 Mar 2023 Sal, 09:03 tarihinde şunu
> yazdı:
>>
>> On Tue, Mar 21, 2023 at 7:03 AM Peter Smith wrote:
>> >
>> >
>> > ==
>&g
On Mon, Mar 20, 2023 at 11:22 PM Jacob Champion wrote:
>
> On Fri, Mar 17, 2023 at 9:45 PM Amit Kapila wrote:
> > > There are a bunch of moving parts and hidden subtleties here, and I fell
> > > into a few traps when I was working on my patch, so it'd be nice to h
On Mon, Mar 20, 2023 at 6:28 PM Amit Kapila wrote:
>
> On Mon, Mar 20, 2023 at 2:58 PM Önder Kalacı wrote:
> >
> >
> > I applied all patches for all the versions, and re-run the subscription
> > tests,
> > all looks good to me.
> >
>
> LGT
n. I feel it would be better without using
disable_on_error option in these tests.
One minor point:
+ format can be faster than the text format, but it is less portable
+ across machine architectures and PostgreSQL versions.
As per email [1], it would be better to use tag here
with PostgreSQL.
[1] - https://www.postgresql.org/message-id/932629.1679322674%40sss.pgh.pa.us
--
With Regards,
Amit Kapila.
On Mon, Mar 20, 2023 at 2:58 PM Önder Kalacı wrote:
>
>
> I applied all patches for all the versions, and re-run the subscription tests,
> all looks good to me.
>
LGTM. I'll push this tomorrow unless there are more comments.
--
With Regards,
Amit Kapila.
On Mon, Mar 20, 2023 at 5:13 PM Tomas Vondra
wrote:
>
> On 3/20/23 12:00, Amit Kapila wrote:
> > On Mon, Mar 20, 2023 at 1:49 PM Tomas Vondra
> > wrote:
> >>
> >>
> >> I don't understand why we'd need WAL from before the slot is created,
&
On Mon, Mar 20, 2023 at 1:49 PM Tomas Vondra
wrote:
>
>
> On 3/20/23 04:42, Amit Kapila wrote:
> > On Sat, Mar 18, 2023 at 8:49 PM Tomas Vondra
> > wrote:
> >>
> >> On 3/18/23 06:35, Amit Kapila wrote:
> >>> On Sat
_true_1, pub_root_true_2;
It is not clear to me what exactly you want to test here. Please add
some comments.
5. I think you can merge the 0001 and 0003 patches.
Apart from the above, attached is a patch to change some of the
comments in the patch.
--
With Regards,
Amit Kapila.
diff
N_PART_ROOT : PUBLICATION_PART_LEAF;
>
IIUC, 2b is an existing code, so I would prefer not to change that as
part of this patch. Similarly, for other comments, unless something is
a very clear improvement and makes difference w.r.t this patch, it
makes sense to change that, otherwise, let's focus on the current
issue.
--
With Regards,
Amit Kapila.
already exists"?
This would be similar to how we deal with conflicting rows with
unique/primary keys.
--
With Regards,
Amit Kapila.
On Sat, Mar 18, 2023 at 8:49 PM Tomas Vondra
wrote:
>
> On 3/18/23 06:35, Amit Kapila wrote:
> > On Sat, Mar 18, 2023 at 3:13 AM Tomas Vondra
> > wrote:
> >>
> >> ...
> >>
> >> Clearly, for sequences we can't quite rely on snapshots/s
to use the subscription
> 'disable_on_error=true' parameter for those cases so that the
> tablesync won't needlessly attempt to relaunch until you have set
> binary=false and then re-enabled the subscription.
>
+1. That would make tests more reliable.
--
With Regards,
Amit Kapila.
gt;
I agree that the previous comment should be updated but I would prefer
something along the lines: "Prior to v16, initial table
synchronization will use text format even if the binary option is
enabled for a subscription."
--
With Regards,
Amit Kapila.
On Thu, Mar 16, 2023 at 2:15 PM Amit Kapila wrote:
>
> On Wed, Mar 15, 2023 at 9:12 AM Amit Kapila wrote:
> >
> > On Tue, Mar 14, 2023 at 3:18 PM Önder Kalacı wrote:
> > >>
> >
> > Pushed this patch but forgot to add a new testfile. Will do that soon.
e. But maybe we could use the page LSN
> from the relfilenode - that should be the LSN of the last WAL record.
>
> Or maybe we could simply add pg_current_wal_insert_lsn() into the SQL we
> use to read the sequence state ...
>
What if some Alter Sequence is performed before the copy starts and
after the copy is finished, the containing transaction rolled back?
Won't it copy something which shouldn't have been copied?
--
With Regards,
Amit Kapila.
> into a few traps when I was working on my patch, so it'd be nice to have
> additional coverage. I'm happy to contribute effort in that area if it's
> helpful.
>
I think it depends on what tests you have in mind. I suggest you can
propose a patch to cover tests for this are in a separate thread. We
can then evaluate those separately.
--
With Regards,
Amit Kapila.
om
[2] -
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2023-03-17%2005%3A36%3A10
--
With Regards,
Amit Kapila.
On Fri, Mar 17, 2023 at 11:58 AM wangw.f...@fujitsu.com
wrote:
>
> On Thu, Mar 16, 2023 at 20:25 PM Amit Kapila wrote:
> >
>
> Thanks for your comments.
>
> > + if (server_version >= 16)
> > + {
> > + appendStringInfo(&
On Fri, Mar 17, 2023 at 5:52 AM Nathan Bossart wrote:
>
> I've attached a minimally-updated patch that doesn't yet address the bigger
> topics under discussion.
>
> On Thu, Mar 16, 2023 at 03:30:37PM +0530, Amit Kapila wrote:
> > On Wed, Feb 1, 2023 at 5:35 AM Nath
On Thu, Mar 16, 2023 at 7:20 AM Peter Smith wrote:
>
> PSA v4 which addresses both of your review comments.
>
Pushed.
--
With Regards,
Amit Kapila.
On Fri, Mar 17, 2023 at 6:42 AM Peter Smith wrote:
>
> On Thu, Mar 16, 2023 at 1:55 PM Amit Kapila wrote:
> >
> > On Wed, Mar 15, 2023 at 3:33 PM Melih Mutlu wrote:
> > >
> > > Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde
> > > şunu yazdı:
> &
On Fri, Mar 17, 2023 at 5:41 AM Tom Lane wrote:
>
> Amit Kapila writes:
> > On Thu, Mar 16, 2023 at 9:33 PM Önder Kalacı wrote:
> >>> and backpatch the fix for dropped column to PG10.
>
> > You can first submit the fix for dropped columns with patches till
>
that is committed, then you can send the patches for
generated columns.
> Or does the committer prefer to handle the conflicts? Depending on your reply,
> I can work on the followup.
>
Thanks for working on it.
--
With Regards,
Amit Kapila.
On Thu, Mar 16, 2023 at 6:59 PM Melih Mutlu wrote:
>
> Amit Kapila , 16 Mar 2023 Per, 06:25 tarihinde şunu
> yazdı:
>>
>> On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira wrote:
>> >
>> > On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
>> >
&g
t (fix when publisher < 16) the
same as HEAD and move the proposed change to a separate patch?
Basically, for the HEAD patch, let's just try to fix this when
publisher >=16. I am slightly worried that as this is a corner case
bug and we didn't see any user complaints for this, so introducing a
complex fix for back branches may not be required or at least we can
discuss that separately.
--
With Regards,
Amit Kapila.
tion"),
...
>
> seq.transactional is true and in_remote_transaction is false. It might
> be an issue of the parallel apply feature rather than this patch.
>
During parallel apply we didn't need to rely on in_remote_transaction,
so it was not set. I haven't checked the patch in detail but am
wondering, isn't it sufficient to instead check IsTransactionState()
and or IsTransactionOrTransactionBlock()?
--
With Regards,
Amit Kapila.
On Wed, Feb 1, 2023 at 5:35 AM Nathan Bossart wrote:
>
> On Sat, Jan 28, 2023 at 10:26:25AM +0530, Amit Kapila wrote:
> > On Fri, Jan 27, 2023 at 4:07 AM Tom Lane wrote:
> >> Returning to the prior patch ... I don't much care for this:
> >>
> >> +
ge will have a link to
the discussion.
Did you check this in the back branches?
--
With Regards,
Amit Kapila.
On Wed, Mar 15, 2023 at 9:12 AM Amit Kapila wrote:
>
> On Tue, Mar 14, 2023 at 3:18 PM Önder Kalacı wrote:
> >>
>
> Pushed this patch but forgot to add a new testfile. Will do that soon.
>
The main patch is committed now. I think the pending item in this
thread is to co
_toptxn instead of an explicit
> variable toptxn for single use?
>
I find all three suggestions are similar. Personally, I think the
current code looks better. The v4 patch LGTM and I am planning to
commit it unless there are more comments or people find your
suggestions as an improvement.
--
With Regards,
Amit Kapila.
is patch, we will still send BEGIN and do OutputPluginWrite, etc.
Also, it will try to perform row_filter when none of old_slot or
new_slot is set. I don't know for which particular case we have s
handling missing old tuples for deletes but that might require changes
in your proposed patch.
--
W
On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira wrote:
>
> On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
>
> It is not clear to me which version check you wanted to add because we
> seem to have a binary option in COPY from the time prior to logical
> replication. I feel w
On Wed, Mar 15, 2023 at 3:33 PM Melih Mutlu wrote:
>
> Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde şunu
> yazdı:
>>
>> On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu wrote:
>
>
>>
>> What purpose does this test serve w.r.t this patch? Before checking
>&
On Thu, Mar 16, 2023 at 5:59 AM Peter Smith wrote:
>
> On Wed, Mar 15, 2023 at 5:31 PM Amit Kapila wrote:
> >
> > On Wed, Mar 15, 2023 at 11:52 AM Peter Smith wrote:
> > >
> > > ==
> > > src/backend/replication/logical/tablesync.c
> > >
t column orders, the patch has already changed
binary to false, so it doesn't seem to test the functionality of this
patch. Am, I missing something?
--
With Regards,
Amit Kapila.
> checking.
>
> Something like this:
>
> SUGGESTION
> If the publisher version is earlier than v14, we use text format COPY.
>
I think this isn't explicit that we supported the binary format since
v14. So, I would prefer my version of the comment as suggested in the
previous email.
--
With Regards,
Amit Kapila.
* support the binary option.
> +*/
>
> This sentence doesn't look correct grammatically. We can replace "it COPY
> command" with "subscription" for example. Kindly please fix it.
>
How about something like: "The binary option for replication is
supported since v14."?
--
With Regards,
Amit Kapila.
On Tue, Mar 14, 2023 at 3:18 PM Önder Kalacı wrote:
>>
Pushed this patch but forgot to add a new testfile. Will do that soon.
--
With Regards,
Amit Kapila.
;txn->toptxn)
> - topxid = change->txn->toptxn->xid;
> + if (rbtxn_is_subtxn(change->txn))
> + topxid = rbtxn_get_toptxn(change->txn)->xid;
>
> If you plan to fix, bothe the above also should be handled.
>
I don't know if it would be any better to change the above two as
compared to what the proposed patch has.
--
With Regards,
Amit Kapila.
done that way.
>
Can't we directly use rbtxn_get_toptxn() for this case? I think that
way code will look neat. I see that it is not exactly matching the
existing check so you might be worried but I feel the new code will
achieve the same purpose and will be easy to follow.
--
With Regards,
Amit Kapila.
with the expression, we will use sequential scan
>>
>>
>
> Thanks, fixed
>
> I'll add the changes to v49 in the next e-mail.
>
It seems you forgot to address these last two comments in the latest version.
--
With Regards,
Amit Kapila.
file was inconsistently using ';' while executing statement with
safe_psql, changed it to remove ';'.
--
With Regards,
Amit Kapila.
v48-0001-Allow-the-use-of-indexes-other-than-PK-and-REPLI.patch
Description: Binary data
d better to write the tests for this feature in the
existing test file 014_binary as that would save some time for node
setup/shutdown and also that would be a more appropriate place for
these tests.
--
With Regards,
Amit Kapila.
that it will pick up a sequence scan. However,
just do the poll for the number of index scans stat once. I think that
will cover the case you are worried about without having a noticeable
impact on test timing.
--
With Regards,
Amit Kapila.
On Mon, Mar 13, 2023 at 2:44 AM Peter Smith wrote:
>
> On Sat, Mar 11, 2023 at 6:05 PM Amit Kapila wrote:
> >
> > On Fri, Mar 10, 2023 at 7:58 PM Önder Kalacı wrote:
> > >
> > >
> > > I think one option could be to drop some cases altogether, but not
1301 - 1400 of 4326 matches
Mail list logo
