On 2020-Oct-06, Amit Kapila wrote:
> I have made a few changes, (a) moved free of missingatts in the caller
> where we are allocating it. (b) added/edited/removed comments, (c) ran
> pgindent.
This is committed as f07707099c17.
Thanks
On Tue, Oct 6, 2020 at 5:24 PM Amit Kapila wrote:
>
> On Tue, Oct 6, 2020 at 12:14 PM Bharath Rupireddy
> wrote:
> >
> > On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila wrote:
> > >
> > >
> > > 3. The patch doesn't seem to be freeing the memory allocated for
> > > missingattsbuf.
> > >
> >
> > I don
On Tue, Oct 6, 2020 at 12:14 PM Bharath Rupireddy
wrote:
>
> On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila wrote:
> >
> >
> > 3. The patch doesn't seem to be freeing the memory allocated for
> > missingattsbuf.
> >
>
> I don't think we need to do that. We are passing missingattsbuf.data to
> erepo
Thanks Amit for the review comments.
On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila wrote:
>
> Few comments:
> ==
> 1.
> + /* Report error with names of the missing localrel column(s). */
> + if (!bms_is_empty(missingatts))
> + {
> + StringInfoData missingattsbuf;
> + intmissingattcnt
On Mon, Oct 5, 2020 at 8:00 PM Bharath Rupireddy
wrote:
>
> Thanks Amit for the review comments. I will post an updated patch soon.
>
> On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila wrote:
> >
> > 6. I think we should add one test case for this in the existing
> > regression test (src/test/subscript
Thanks Amit for the review comments. I will post an updated patch soon.
On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila wrote:
>
> 6. I think we should add one test case for this in the existing
> regression test (src/test/subscription/t/008_diff_schema).
>
This patch logs the missing column names me
On Wed, Sep 16, 2020 at 9:58 PM Bharath Rupireddy
wrote:
>
> Attaching v6 patch, rebased because of a recent commit
> 3d65b0593c5578014f62e09d4008006f1783f64d. Please consider this for
> further review.
>
Few comments:
==
1.
+ /* Report error with names of the missing localrel column(
Attaching v6 patch, rebased because of a recent commit
3d65b0593c5578014f62e09d4008006f1783f64d. Please consider this for
further review.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
v6-0001-Detail-message-with-names-of-missing-columns-in-l.patch
Description: Binary
On Fri, Sep 11, 2020 at 9:05 PM Alvaro Herrera wrote:
>
> On 2020-Sep-11, Tom Lane wrote:
>
> > Check, but you could imagine that the column-list string is constructed
> > with code along the lines of
> >
> > if (first)
> > appendStringInfo(buf, _("\"%s\""), colname);
> > els
Alvaro Herrera writes:
> This works OK for my language at least. I couldn't find any guidance on
> whether there's a problem doing things this way for RTL languages etc,
> but +1 for doing it this way.
Hmm ... fortunately, there's not any large semantic significance to the
order in which the col
On 2020-Sep-11, Tom Lane wrote:
> Check, but you could imagine that the column-list string is constructed
> with code along the lines of
>
> if (first)
> appendStringInfo(buf, _("\"%s\""), colname);
> else
> appendStringInfo(buf, _(", \"%s\""), colname);
> thus all
Alvaro Herrera writes:
> On 2020-Sep-11, Tom Lane wrote:
>> NO. The convention is to write \"...\" in the translatable message.
> There is a problem here though, which is that the quoted strings in
> question are part of a list of columns. There's no way to keep that
> list as a translatable st
On 2020-Sep-11, Tom Lane wrote:
> > How about quoting all the individual columns? Looks like quote_identifier()
> > doesn't serve our purpose here as it selectively quotes or quotes all
> > identifiers only in case quote_all_identifiers config variable is set.
>
> NO. The convention is to write
Bharath Rupireddy writes:
>> I think we always double-quote identifiers in error messages. For
>> example:
>> ./catalog/heap.c 706: errmsg("no collation was derived for column \"%s\"
>> with collatable type %s",
Right. Anything in this patch that is not doing that needs to be fixed.
(As this ex
Thanks for the review comments. Attaching v4 patch.
On Fri, Sep 11, 2020 at 6:44 AM Kyotaro Horiguchi
wrote:
>
> + /* remoterel doesn't contain dropped attributes, see */
> - found = 0;
> + missing_attr = bms_add_range(NULL, 0, remoterel->natts - 1);
> for (i = 0; i < desc->natts; i++)
>
Thanks for the revised version.
At Tue, 8 Sep 2020 22:36:17 +0530, Bharath Rupireddy
wrote in
> Thanks for the comments. Attaching the v3 patch.
>
> On Tue, Sep 8, 2020 at 8:19 PM Alvaro Herrera
> wrote:
> >
> > This looks a bit fiddly. Would it be less cumbersome to use
> > quote_identifie
Added this to the commitfest - https://commitfest.postgresql.org/30/2727/
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Thanks for the comments. Attaching the v3 patch.
On Tue, Sep 8, 2020 at 8:19 PM Alvaro Herrera wrote:
>
> This looks a bit fiddly. Would it be less cumbersome to use
> quote_identifier here instead?
>
Changed. quote_identifier() adds quotes wherever necessary.
>
> Please do use errmsg_plural -
On 2020-Sep-08, Bharath Rupireddy wrote:
> + /* Find the remote attributes that are missing in the local relation. */
> + for (i = 0; i < remoterel->natts; i++)
> + {
> + if (!bms_is_member(i, localattnums))
> + {
> + if (missingatts->len ==
Thanks for the comments, v2 patch is attached.
>
> On Tue, Sep 8, 2020 at 6:50 AM Kyotaro Horiguchi
> wrote:
> >
> > +1 for objective. However, that can be done simpler way that doesn't
> > need additional loops by using bitmapset to hold missing remote
> > attribute numbers. This also make the v
On Tue, Sep 8, 2020 at 6:50 AM Kyotaro Horiguchi
wrote:
>
> +1 for objective. However, that can be done simpler way that doesn't
> need additional loops by using bitmapset to hold missing remote
> attribute numbers. This also make the variable "found" useless.
>
Thanks. I will look into it and po
Thank you for working on this.
At Mon, 7 Sep 2020 16:30:59 +0530, Bharath Rupireddy
wrote in
> Hi,
>
> I observed that, in logical replication when a subscriber is missing
> some columns, it currently emits an error message that says "some"
> columns are missing(see logicalrep_rel_open()), but
Hi,
I observed that, in logical replication when a subscriber is missing
some columns, it currently emits an error message that says "some"
columns are missing(see logicalrep_rel_open()), but it doesn't specify
what the missing column names are. The comment there also says that
finding the missing
23 matches
Mail list logo
