On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
wrote:
>
> On 10 Feb 2021, at 06:32, Amit Kapila wrote:
> >
> > On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
> >>
> >> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
> >>>
> >>
>
On Thu, Feb 11, 2021 at 3:20 PM Petr Jelinek
wrote:
>
> On 11 Feb 2021, at 10:42, Amit Kapila wrote:
> >
> > On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
> > wrote:
> >>
> >> On 10 Feb 2021, at 06:32, Amit Kapila wrote:
> >>>
&g
because of inconsistent snapshot but a forward location in
start_decoding_at. If you don't involve session-4, then it will always
skip prepare due to an inconsistent snapshot state. This involves a
debugger so not easy to write an automated test for it.
I have used a bit tricky scenario to explain this but not sure if
there was any other simpler way.
[1] -
https://www.postgresql.org/message-id/CAA4eK1%2BLvkeX%3DB3xon7RcBwD4CVaFSryPj3pTBAALrDxQVPDwA%40mail.gmail.com
--
With Regards,
Amit Kapila.
On Thu, Feb 11, 2021 at 3:32 PM Petr Jelinek
wrote:
>
> On 11 Feb 2021, at 10:56, Amit Kapila wrote:
> >
> >> Well, I was thinking it could be NOTICE or LOG to be honest, WARNING seems
> >> unnecessarily scary for those usecases to me.
> >>
> >
>
On Thu, Dec 2, 2021 at 4:51 AM Greg Nancarrow wrote:
>
> On Wed, Dec 1, 2021 at 10:15 PM Amit Kapila wrote:
> >
>
> Also, perhaps the following additional comment (or similar) could be
> added to the pg_publication_tables documentation in catalogs.sgml:
>
> For publica
On Thu, Dec 2, 2021 at 6:35 AM osumi.takami...@fujitsu.com
wrote:
>
> On Wednesday, December 1, 2021 10:16 PM Amit Kapila
> wrote:
> Updated the patch to include the notification.
>
The patch disables the subscription for non-transient errors. I am not
sure if we can easily m
cluded in the view, whereas if publish_via_partition_root is set to
> false, only the individual partitions are included in the view.
>
Yeah, that sounds good to me.
--
With Regards,
Amit Kapila.
On Thu, Dec 2, 2021 at 8:42 AM Amit Langote wrote:
>
> On Thu, Dec 2, 2021 at 9:44 houzj.f...@fujitsu.com
> wrote:
> > On Wed, Dec 1, 2021 3:01 PM Amit Kapila wrote:
> > > > > On Mon, Nov 22, 2021 at 12:55 PM Amit Langote
> > > > > wrote:
&
but not sure if that is a
good option.
Thoughts?
--
With Regards,
Amit Kapila.
On Thu, Dec 2, 2021 at 12:51 AM Mark Dilger
wrote:
>
>
> > On Dec 1, 2021, at 5:36 AM, Amit Kapila wrote:
> >
> > On Wed, Dec 1, 2021 at 2:12 AM Jeff Davis wrote:
> >>
> >> On Tue, 2021-11-30 at 17:25 +0530, Amit Kapila wrote:
> >>&g
On Thu, Dec 2, 2021 at 8:38 PM Peter Eisentraut
wrote:
>
> On 02.12.21 07:48, Amit Kapila wrote:
> > a. ALTER SUBSCRIPTION ... [SET|RESET] SKIP TRANSACTION xxx;
> > b. Alter Subscription SET ( subscription_parameter [=value]
> > [, ... ] );
> > c
On Thu, Dec 2, 2021 at 7:18 PM Amit Langote wrote:
>
> On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila wrote:
> > On Thu, Dec 2, 2021 at 8:42 AM Amit Langote wrote:
> > > Reading Alvaro's email at the top again gave me a pause to reconsider
> > > what I had said i
anup conditionally,
+ * but we have already processed the index (for bulkdelete). We do
+ * this to avoid the need to invoke workers when parallel index
+ * cleanup doesn't need to scan the index. See the comments for
+ * option VACUUM_OPTION_PARALLEL_COND_CLEANUP to know when indexes
+ * support parallel cleanup conditionally.
+ */
--
With Regards,
Amit Kapila.
omment to remove the call to
parallel_vacuum_should_skip_index() from
parallel_vacuum_index_is_parallel_safe(). If Sawada-San follows that
then maybe your point will be addressed.
--
With Regards,
Amit Kapila.
On Fri, Dec 3, 2021 at 2:33 PM Amit Kapila wrote:
>
> On Thu, Dec 2, 2021 at 6:01 PM Masahiko Sawada wrote:
> >
> > I've attached updated patches.
> >
>
> I have a few comments on v4-0001.
>
The new test proposed by v4-0003 is increasing the vacuum_paralle
On Fri, Dec 3, 2021 at 6:06 PM Masahiko Sawada wrote:
>
> On Fri, Dec 3, 2021 at 6:03 PM Amit Kapila wrote:
> >
> > On Thu, Dec 2, 2021 at 6:01 PM Masahiko Sawada
> > wrote:
> > >
> > > I've attached updated patches.
> > >
> &g
On Fri, Dec 3, 2021 at 6:04 PM Amit Langote wrote:
>
> On Fri, Dec 3, 2021 at 12:37 PM Amit Kapila wrote:
> > On Thu, Dec 2, 2021 at 7:18 PM Amit Langote wrote:
> > > On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila
> > > wrote:
> > > > On Thu, Dec 2, 20
On Fri, Dec 3, 2021 at 12:12 PM Masahiko Sawada wrote:
>
> On Fri, Dec 3, 2021 at 11:53 AM Amit Kapila wrote:
> >
> > > But this syntax gives you flexibility, so we can also
> > > start with a simple implementation.
> > >
> >
> > Yeah, I also
ches are done,
otherwise, we need to rethink how to divide for easier review. We need
to retain the 0005 patch because that handles many problems without
which the main patch is incomplete and buggy w.r.t replica identity.
[1] -
https://www.postgresql.org/message-id/CAA4eK1%2BXoD49bz5-2TtiD0ugq4PHSRX2D1sLPR_X4LNtdMc4OQ%40mail.gmail.com
--
With Regards,
Amit Kapila.
o review that bug-fix patch and the 0003 patch being
discussed here [2] to avoid missing any already reported issues in
this thread?
[1] - https://commitfest.postgresql.org/36/3162/
[2] -
https://www.postgresql.org/message-id/CAHut%2BPtJnnM8MYQDf7xCyFAp13U_0Ya2dv-UQeFD%3DghixFLZiw%40mail.gmail.com
--
With Regards,
Amit Kapila.
bit weird because of
> that, and inconsistent with the existing
> puballtables/pg_publication_rel.
>
What do you mean by inconsistent with puballtables/pg_publication_rel?
--
With Regards,
Amit Kapila.
On Mon, Dec 6, 2021 at 1:00 PM Amit Langote wrote:
>
> On Mon, Dec 6, 2021 at 3:55 PM houzj.f...@fujitsu.com
> wrote:
> > On Thursday, December 2, 2021 9:48 PM Amit Langote
> > wrote:
> > > On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila
> > > wrote:
>
ry. I had a similar code
> in a ancient version of this patch but decided that the additional code is not
> worth.
>
> There is at least one issue in the current code that should be addressed: PK
> or
> REPLICA IDENTITY modification could break the publication check for UPDATEs
> and
> DELETEs.
>
Please see patch 0005 [1]. I think it tries to address the issues
w.r.t Replica Identity interaction with this feature. Feel free to
test/review and let us know if you see any issues.
[1] -
https://www.postgresql.org/message-id/CAHut%2BPtJnnM8MYQDf7xCyFAp13U_0Ya2dv-UQeFD%3DghixFLZiw%40mail.gmail.com
--
With Regards,
Amit Kapila.
On Fri, Dec 3, 2021 at 10:37 PM Mark Dilger
wrote:
>
> > On Dec 2, 2021, at 1:29 AM, Amit Kapila wrote:
> >
> > If we want to maintain the property that subscriptions can only be
> > owned by superuser for your first version then isn't a simple check
> >
On Mon, Dec 6, 2021 at 10:07 AM Mark Dilger
wrote:
>
> > On Dec 1, 2021, at 8:48 PM, Amit Kapila wrote:
> >
> > The patch disables the subscription for non-transient errors. I am not
> > sure if we can easily make the call to decide whether any particular
> &g
On Mon, Dec 6, 2021 at 6:18 PM Euler Taveira wrote:
>
> On Mon, Dec 6, 2021, at 3:44 AM, Amit Kapila wrote:
>
> True, but that is the main reason the review and development are being
> done as separate sub-features. I suggest still keeping the similar
> separation till some of t
On Tue, Dec 7, 2021 at 6:54 AM Masahiko Sawada wrote:
>
> On Mon, Dec 6, 2021 at 1:47 PM Amit Kapila wrote:
> >
> > On Fri, Dec 3, 2021 at 6:06 PM Masahiko Sawada
> > wrote:
> > >
> > > >
> > > > 3. /*
> > > >
u think? I am fine if you think that we should evaluate/analyze
your proposal before making a call on whether to fix the existing
issues with the proposed set of patches.
[1] -
https://www.postgresql.org/message-id/OS0PR01MB57165D94CB187A97128F75EA946A9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
On Mon, Dec 6, 2021 at 8:59 PM Amit Langote wrote:
>
> On Mon, Dec 6, 2021 at 6:05 PM Amit Kapila wrote:
> > On Mon, Dec 6, 2021 at 1:00 PM Amit Langote wrote:
> > > On Mon, Dec 6, 2021 at 3:55 PM houzj.f...@fujitsu.com
> > > wrote:
> > > > After th
p to both the
> replication origin timestamp and the error callback argument on the
> subscriber. I've attached the patch to fix it.
>
Thanks for the report and patches. I see this is a problem and the
first patch will fix it. I'll test the same and review another patch
as well.
--
With Regards,
Amit Kapila.
On Tue, Dec 7, 2021 at 10:52 AM Amit Langote wrote:
>
> On Tue, Dec 7, 2021 at 1:01 PM Amit Kapila wrote:
> > On Mon, Dec 6, 2021 at 8:59 PM Amit Langote wrote:
> > > On Mon, Dec 6, 2021 at 6:05 PM Amit Kapila
> > > wrote:
> > > > Your concern is not
ons or
objections about this?
--
With Regards,
Amit Kapila.
v4-0001-Fix-changing-the-ownership-of-ALL-TABLES-IN-SCHEM.patch
Description: Binary data
On Mon, Dec 6, 2021 at 9:26 PM Mark Dilger wrote:
>
> > On Dec 6, 2021, at 2:19 AM, Amit Kapila wrote:
> >
> >>> If we want to maintain the property that subscriptions can only be
> >>> owned by superuser
>
> We don't want to maintain such a pr
On Tue, Dec 7, 2021 at 5:06 PM Masahiko Sawada wrote:
>
> On Mon, Dec 6, 2021 at 2:17 PM Amit Kapila wrote:
>
> I'll submit the patch tomorrow.
>
> While updating the patch, I realized that skipping a transaction that
> is prepared on the publisher will be tricky a bi
t_rel_sync_entry(), we already have this information, can't we
someway stash that in the corresponding RelationSyncEntry so that same
can be used later for row filtering.
> Instead of that can we add a "TRUE" filter on all the tables
> which are part of FOR ALL TABLES publication?
>
How? We won't have an entry for such tables in pg_publication_rel
where we store row_filter information.
--
With Regards,
Amit Kapila.
or not as this is not a
new feature and even if we need it as an improvement to docs, shall we
consider backpatching it? I felt that code changes are required to fix
a known issue so the case of backpatching it is clear.
--
With Regards,
Amit Kapila.
On Wed, Dec 8, 2021 at 11:30 AM vignesh C wrote:
>
> On Wed, Dec 8, 2021 at 11:11 AM Amit Kapila wrote:
> >
> > On Tue, Dec 7, 2021 at 5:53 PM vignesh C wrote:
> > >
> > > On Fri, Dec 3, 2021 at 11:24 AM houzj.f...@fujitsu.com
> > > wrote:
> &g
d is to evaluate the
performance for both cases (when we apply a filter to both tuples vs.
to one of the tuples). In case the performance difference is
unacceptable, I think it would be better to still compare both tuples
as default to avoid data inconsistency issues and have an option to
allow comparing one of the tuples.
--
With Regards,
Amit Kapila.
On Wed, Dec 8, 2021 at 11:48 AM Masahiko Sawada wrote:
>
> On Wed, Dec 8, 2021 at 2:16 PM Amit Kapila wrote:
> >
> > On Tue, Dec 7, 2021 at 5:06 PM Masahiko Sawada
> > wrote:
> > >
> > > On Mon, Dec 6, 2021 at 2:17 PM Amit Kapila
> > >
On Wed, Dec 8, 2021 at 12:36 PM Masahiko Sawada wrote:
>
> On Wed, Dec 8, 2021 at 3:50 PM Amit Kapila wrote:
> >
> > On Wed, Dec 8, 2021 at 11:48 AM Masahiko Sawada
> > wrote:
> > >
> > > >
> > > > Can't we think of just allowing
On Tue, Dec 7, 2021 at 11:20 PM Bossart, Nathan wrote:
>
> On 12/7/21, 2:47 AM, "Greg Nancarrow" wrote:
> > On Tue, Dec 7, 2021 at 9:01 PM Amit Kapila wrote:
> >>
> >> Thanks, the patch looks mostly good to me. I have slightly modified it
> >>
On Thu, Dec 2, 2021 at 8:42 AM Amit Langote wrote:
>
> On Thu, Dec 2, 2021 at 9:44 houzj.f...@fujitsu.com
> wrote:
> > On Wed, Dec 1, 2021 3:01 PM Amit Kapila wrote:
> > > > > On Mon, Nov 22, 2021 at 12:55 PM Amit Langote
> > > > > wrote:
&
scriber relies on them. I think for the sake of
consistency it is better to check the validity of lsn's but if we want
to add for others, we should analyze and add for all the required ones
in one shot.
--
With Regards,
Amit Kapila.
iously suggested by Peter Smith
> and the idea wasn't looked upon all that favorably?
>
I think if we are really worried about transient errors then probably
the idea "disable only if the same error has occurred more than X
times" seems preferable as compared to taking a decision on which
error_codes fall in the transient error category.
--
With Regards,
Amit Kapila.
On Wed, Dec 8, 2021 at 4:36 PM Masahiko Sawada wrote:
>
> On Wed, Dec 8, 2021 at 5:54 PM Amit Kapila wrote:
> >
> > On Wed, Dec 8, 2021 at 12:36 PM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Dec 8, 2021 at 3:50 PM Amit Kapila
> > > wro
On Wed, Dec 8, 2021 at 9:22 PM Mark Dilger wrote:
>
>
> > On Dec 8, 2021, at 5:10 AM, Amit Kapila wrote:
> >
> > I think if we are really worried about transient errors then probably
> > the idea "disable only if the same error has occurred more than X
> &g
On Tue, Dec 7, 2021 at 8:25 PM Mark Dilger wrote:
>
> > On Dec 7, 2021, at 2:29 AM, Amit Kapila wrote:
> >
> > Okay, let me try to explain again. Following is the text from docs
> > [1]: " (a) To create a subscription, the user must be a superuser. (b)
> > T
On Thu, Dec 9, 2021 at 2:24 PM Masahiko Sawada wrote:
>
> On Thu, Dec 9, 2021 at 11:47 AM Amit Kapila wrote:
> >
> > I am thinking that we can start a transaction, update the catalog,
> > commit that transaction. Then start a new one to update
> > origin_lsn/timesta
On Mon, Dec 6, 2021 at 10:17 AM Amit Kapila wrote:
>
> On Fri, Dec 3, 2021 at 6:06 PM Masahiko Sawada wrote:
> >
> > On Fri, Dec 3, 2021 at 6:03 PM Amit Kapila wrote:
> > >
> > > On Thu, Dec 2, 2021 at 6:01 PM Masahiko Sawada
> > > wrote:
On Thu, Dec 9, 2021 at 3:35 PM Amit Kapila wrote:
>
> On Mon, Dec 6, 2021 at 10:17 AM Amit Kapila wrote:
> >
> > On Fri, Dec 3, 2021 at 6:06 PM Masahiko Sawada
> > wrote:
> > > > 2. The patch seems to be calling parallel_vacuum_should_skip_index
> >
On Wed, Dec 8, 2021 at 11:38 AM Amit Kapila wrote:
>
> On Wed, Dec 8, 2021 at 11:30 AM vignesh C wrote:
> >
> > On Wed, Dec 8, 2021 at 11:11 AM Amit Kapila wrote:
> > >
> > > On Tue, Dec 7, 2021 at 5:53 PM vignesh C wrote:
> > > >
> > >
ll also in turn increase the scan
cost where we need only schema or rel publication access like where
ever we are using PUBLICATIONNAMESPACEMAP. I think it will increase
the cost of scanning table publications as its corresponding index
will be larger.
--
With Regards,
Amit Kapila.
On Thu, Dec 9, 2021 at 10:52 PM Mark Dilger
wrote:
>
> > On Dec 9, 2021, at 7:41 AM, Robert Haas wrote:
> >
> > On Tue, Nov 30, 2021 at 6:55 AM Amit Kapila wrote:
> >>> This patch does detect ownership changes more quickly (at the
> >>> transaction
On Thu, Dec 9, 2021 at 6:05 PM Masahiko Sawada wrote:
>
> On Thu, Dec 9, 2021 at 7:44 PM Amit Kapila wrote:
>
> Agreed with the above two points.
>
> I've attached updated patches that incorporated the above comments
> too. Please review them.
>
I have made the f
On Fri, Dec 10, 2021 at 7:39 PM Robert Haas wrote:
>
> On Thu, Dec 9, 2021 at 11:15 PM Amit Kapila wrote:
> > Yeah, to me also (b) sounds better than (a). However, a few points
> > that we might want to consider in that regard are as follows: 1.
> > locking the subscript
On Fri, Dec 10, 2021 at 11:14 AM Masahiko Sawada wrote:
>
> On Thu, Dec 9, 2021 at 6:16 PM Amit Kapila wrote:
> >
> > On Thu, Dec 9, 2021 at 2:24 PM Masahiko Sawada
> > wrote:
> > >
> > > On Thu, Dec 9, 2021 at 11:47 AM Amit Kapila
> > > wro
o?
>
> Yes, we cannot know whether we can actually start workers when
> starting parallel index vacuuming. It returns non-NULL if we request
> one or more workers.
>
So can we adjust the comments? I think the part of the sentence "when
we can launch one or more workers" seems to be the cause of confusion,
can we remove it?
--
With Regards,
Amit Kapila.
On Mon, Dec 13, 2021 at 8:28 AM Masahiko Sawada wrote:
>
> On Sat, Dec 11, 2021 at 3:29 PM Amit Kapila wrote:
> >
> > 3.
> > + * Also, we don't skip receiving the changes in streaming cases,
> > since we decide
> > + * whether or not to skip ap
On Mon, Dec 13, 2021 at 10:33 AM Masahiko Sawada wrote:
>
> On Fri, Dec 10, 2021 at 9:08 PM Amit Kapila wrote:
> >
> > On Thu, Dec 9, 2021 at 6:05 PM Masahiko Sawada
> > wrote:
> > >
> > > On Thu, Dec 9, 2021 at 7:44 PM Amit Kapila
> > >
Regards,
Amit Kapila.
e replication origin in the commit and rollback code
> paths.
>
This is my understanding as well. I think here the point of Sawada-San
is why to have additional for replorigin_session_origin_lsn in prepare
code path? I think the way you have it in your patch is correct as
well but it is probabl
On Mon, Dec 13, 2021 at 5:48 PM osumi.takami...@fujitsu.com
wrote:
>
> On Monday, December 13, 2021 6:19 PM Amit Kapila
> wrote:
> > On Tue, Dec 7, 2021 at 3:12 PM osumi.takami...@fujitsu.com
> > wrote:
> >
> > Few questions and commen
On Mon, Dec 13, 2021 at 6:55 PM Masahiko Sawada wrote:
>
> On Mon, Dec 13, 2021 at 1:04 PM Amit Kapila wrote:
> >
> > On Mon, Dec 13, 2021 at 8:28 AM Masahiko Sawada
> > wrote:
> >
> > > >
> > > > 4.
> > > > + * Also, one might
On Tue, Dec 14, 2021 at 7:40 AM tanghy.f...@fujitsu.com
wrote:
>
> On Monday, December 13, 2021 2:12 PM Masahiko Sawada
> wrote:
> >
> > On Mon, Dec 13, 2021 at 2:09 PM Amit Kapila wrote:
> > >
> > > On Mon, Dec 13, 2021 at 10:33 AM Masahiko Sawada
>
* filter absence means replicate all rows so a single valid
+ * expression means publish this row.
This same comment is at two places, remove from one of the places. I
think keeping it atop for loop is better.
3.
+ {
+ int idx;
+ bool found_filters = false;
I am not sure if starting such ad-hoc braces in the code to localize
the scope of variables is a regular practice. Can we please remove
this?
--
With Regards,
Amit Kapila.
On Tue, Dec 14, 2021 at 1:07 PM Dilip Kumar wrote:
>
> On Tue, Dec 14, 2021 at 8:20 AM Amit Kapila wrote:
> >
> > On Mon, Dec 13, 2021 at 6:55 PM Masahiko Sawada
> > wrote:
>
> > > In streaming cases, we don’t know when stream-commit or stream-abort
> >
On Tue, Dec 14, 2021 at 10:50 AM Amit Kapila wrote:
>
> On Tue, Dec 14, 2021 at 4:44 AM Peter Smith wrote:
>
> Few other comments:
> ===
Few more comments:
==
v46-0001/0002
===
1. After rowfilter_walker() why do we need
EXPR_KIND_PUBLI
On Tue, Dec 14, 2021 at 3:41 PM Dilip Kumar wrote:
>
> On Tue, Dec 14, 2021 at 2:36 PM Amit Kapila wrote:
> >
> > > >
> > > > I agree with this theory. Can we reflect this in comments so that in
> > > > the future we know why we didn't
On Wed, Dec 15, 2021 at 8:23 AM Peter Geoghegan wrote:
>
> On Mon, Dec 13, 2021 at 7:03 PM Amit Kapila wrote:
> > Thanks, I can take care of this before committing. The v9-0001* looks
> > good to me as well, so, I am planning to commit that tomorrow unless I
> >
On Wed, Dec 15, 2021 at 6:47 AM Peter Smith wrote:
>
> On Tue, Dec 14, 2021 at 10:12 PM Amit Kapila wrote:
> >
> > On Tue, Dec 14, 2021 at 10:50 AM Amit Kapila
> > wrote:
> > >
> > > On Tue, Dec 14, 2021 at 4:44 AM Peter Sm
ng it for add/update rel state (see,
AddSubscriptionRelState, UpdateSubscriptionRelState), so this will be
another place to use a similar technique.
--
With Regards,
Amit Kapila.
ceed. We have discussed providing these additional options in this
thread but thought of doing it later once we have the base feature and
based on the feedback from users.
--
With Regards,
Amit Kapila.
not sure what you have in mind here? How can we change the
already released code pre-15 for this new feature?
--
With Regards,
Amit Kapila.
On Wed, Dec 15, 2021 at 1:52 PM Greg Nancarrow wrote:
>
> On Wed, Dec 15, 2021 at 5:25 PM Amit Kapila wrote:
> >
> > > "If a subscriber is a pre-15 version, the initial table
> > > synchronization won't use row filters even if they are defined in the
>
ted :-/
>
By any chance, will this impact synchronous replication as well which
waits for commits to be replicated?
How is this patch dealing with prepared transaction case where at
prepare we will send transactional changes and then later if rollback
prepared happens then the publisher will rollback changes related to
new relfilenode but subscriber would have already replayed the updated
seqval change which won't be rolled back?
--
With Regards,
Amit Kapila.
On Wed, Dec 15, 2021 at 8:19 PM Masahiko Sawada wrote:
>
> On Wed, Dec 15, 2021 at 1:10 PM Amit Kapila wrote:
> >
> > On Wed, Dec 15, 2021 at 8:19 AM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Dec 14, 2021 at 2:35 PM Greg Nancarrow
> > &g
ivinfo.message_level = elevel;
-
ivinfo.num_heap_tuples = reltuples;
This seems like an unrelated change.
--
With Regards,
Amit Kapila.
On Thu, Dec 16, 2021 at 10:37 AM Masahiko Sawada wrote:
>
> On Thu, Dec 16, 2021 at 11:43 AM Amit Kapila wrote:
> >
> > I thought we just want to lock before clearing the skip_xid something
> > like take the lock, check if the skip_xid in the catalog is the same
> >
On Wed, Dec 15, 2021 at 7:21 PM Tomas Vondra
wrote:
>
> On 12/15/21 14:20, Amit Kapila wrote:
> > On Tue, Dec 14, 2021 at 7:02 AM Tomas Vondra
> > wrote:
> >>
> >> Hi,
> >>
> >> here's an updated version of the patches, dealing with a
On Thu, Dec 16, 2021 at 6:13 PM Masahiko Sawada wrote:
>
> On Thu, Dec 16, 2021 at 1:56 PM Amit Kapila wrote:
> >
> > On Wed, Dec 15, 2021 at 1:33 PM Masahiko Sawada
> > wrote:
> > >
> > > I've attached an updated patch. The patch incorpora
hat on the lines of what row_filter
patch is also doing where if the user drops the column that was part
of row_filter for a relation in publication, we give an error and if
the user tries to drop the column with CASCADE then the relation is
removed from the publication.
--
With Regards,
Amit Kapila.
cts like
RelIdCacheEnt.
6. Can we name IsRowFilterSimpleNode() as IsRowFilterSimpleExpr()?
--
With Regards,
Amit Kapila.
>
+1 for using SKIP (xid = NONE) because otherwise first we need to
introduce RESET syntax for this command.
--
With Regards,
Amit Kapila.
published or not?
> I think it's futile for the publisher side to try and figure out the
> history of published rows.
>
I also think so. One more thing, even if we want we might not be able
to apply the insert filter as the corresponding values may not be
logged.
--
With Regards,
Amit Kapila.
ues of row which is what I think we are expecting here.
> Doesn't this problem result from allowing different WHERE clauses for
> different pubactions for the same table?
> My current thoughts are that this shouldn't be allowed, and also WHERE
> clauses for INSERTs should,
set of
columns or (b) when the columns contain some sensitive information
like personal identification number, etc. I think there could be a
side benefit in this which comes from the fact that the lesser data
will flow across the network which could lead to faster replication
especially when the user filters large column data.
--
With Regards,
Amit Kapila.
nd DELETE every time, we can also save the result in the
> relcache. It's safe because every operation change the RI will invalidate the
> relcache. We are using this approach in row filter patch to make sure all
> columns in row filter expression are part of RI.
>
Another point related to RI is that this patch seems to restrict
specifying the RI columns in the column filter list irrespective of
publish action. Do we need to have such a restriction if the
publication publishes 'insert' or 'truncate'?
--
With Regards,
Amit Kapila.
lVacuumIsActive(vacrel));
I think we can retain the older Assert. If we do that then I think we
don't need to define ParallelVacuumIsActive in vacuumlazy.c.
--
With Regards,
Amit Kapila.
ROLLBACK;
>
>BEGIN;
>SELECT nextval('s');
>COMMIT;
>
> The natural expectation would be the COMMIT gets stuck, waiting for the
> sync replica (which is not running), right? But it does not.
>
How about if we always WAL log the first sequence change in a transaction?
--
With Regards,
Amit Kapila.
On Mon, Dec 20, 2021 at 6:07 AM Greg Nancarrow wrote:
>
> On Sat, Dec 18, 2021 at 1:33 PM Amit Kapila wrote:
> >
> > >
> > > I think it's a concern, for such a basic example with only one row,
> > > getting unpredictable (and even wrong) replicatio
On Mon, Dec 20, 2021 at 8:33 AM Masahiko Sawada wrote:
>
> On Sat, Dec 18, 2021 at 3:38 PM Amit Kapila wrote:
> >
> > Few comments:
> > =
> > 1.
> > + * dead_items stores TIDs whose index tuples are deleted by index
> > vacuuming.
> >
ge I see with this is that we will avoid adding
additional columns for the other patches like "column filter". Also,
it might be convenient for users. What do you think?
--
With Regards,
Amit Kapila.
sion to merge it once we are done with our evaluation.
--
With Regards,
Amit Kapila.
On Mon, Dec 20, 2021 at 6:29 PM Masahiko Sawada wrote:
>
> On Mon, Dec 20, 2021 at 1:08 PM Amit Kapila wrote:
> >
> > > >
> > > > 2. What is the reason for not moving
> > > > lazy_vacuum_one_index/lazy_cleanup_one_index to vacuum.c so that
gicalrep_write_tuple and 3 new
> entries into RelationSyncEntry). I replaced this patch with a slightly
> different one (0005 in this patch set) that uses HeapTuple instead. I didn't
> only simple tests and it requires tests. I noticed that this patch does not
> include a test to cover the case where TOASTed values are not included in the
> new tuple. We should probably add one.
>
Yeah, it would be good to add such a test.
--
With Regards,
Amit Kapila.
On Tue, Dec 21, 2021 at 10:05 AM Masahiko Sawada wrote:
>
> On Tue, Dec 21, 2021 at 12:05 PM Amit Kapila wrote:
> >
> > On Mon, Dec 20, 2021 at 6:29 PM Masahiko Sawada
> > wrote:
>
> > BTW, if we go with that then we should set the correct phase
> &
is quite off. Please run pgindent.
5. If we decide to go with this approach then I feel let's merge the
required comments from Euler's version.
--
With Regards,
Amit Kapila.
On Tue, Dec 21, 2021 at 10:53 AM vignesh C wrote:
>
> On Mon, Dec 20, 2021 at 8:41 AM houzj.f...@fujitsu.com
> wrote:
> >
> > On Fri, Dec 17, 2021 6:09 PM Amit Kapila wrote:
> > > On Fri, Dec 17, 2021 at 4:11 AM Peter Smith wrote:
> > > >
> >
On Tue, Dec 21, 2021 at 11:24 AM Masahiko Sawada wrote:
>
> On Tue, Dec 21, 2021 at 2:04 PM Amit Kapila wrote:
> >
>
> Thank you for the comment. Agreed.
>
> I've attached updated version patches. Please review them.
>
These look mostly good to me. Please f
-> Seq Scan on
> pg_publication_namespace pn (cost=0.00..0.00 rows=1 width=4) (actual
> time=0.005..0.005 rows=0 loops=1)
> -> Seq Scan on pg_publication p
> (cost=0.00..1.96 rows=1 width=4) (actual time=0.007..0.007 rows=1
> loops=1)
>Filter: (pubname = 'pub1'::name)
> Planning Time: 1.067 ms
> Execution Time: 0.431 ms
> (25 rows)
>
> Combining existing query to include NOT EXISTS based on Euler's
> changes seems to be better:
> SELECT DISTINCT pg_get_expr(prqual, prrelid) FROM pg_publication p
> INNER JOIN pg_publication_rel pr ON (p.oid = pr.prpubid)
> WHERE pr.prrelid = 16384 AND p.pubname IN ( 'pub1' )
> AND NOT (select bool_or(puballtables)
> FROM pg_publication
> WHERE pubname in ( 'pub1' ))
> AND NOT EXISTS (SELECT 1
> FROM pg_publication_namespace pn, pg_class c
> WHERE c.oid = 16384 AND c.relnamespace = pn.pnnspid);
>
The modified query proposed by you seems better to me based on time.
--
With Regards,
Amit Kapila.
601 - 700 of 6713 matches
Mail list logo
