Thanks for the revised version.
At Tue, 8 Sep 2020 22:36:17 +0530, Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote in
> Thanks for the comments. Attaching the v3 patch.
>
> On Tue, Sep 8, 2020 at 8:19 PM Alvaro Herrera <alvhe...@2ndquadrant.com>
> 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 -- have the new function return the number
> > of missing columns. Should be pretty straightforward.
> >
>
> Changed. Now the error message looks as below:
^^;
I don't think the logicalrep_find_missing_attrs worth a separate
function. The detection code would be short enough to be embedded in
the checking loop in logicalrep_rel_open. Note that remoterel doesn't
have missing columns since they are already removed when it is
constructed. See fetch_remote_table_info and
logicalrep_rel_att_by_name is relying on that fact. As the result
this code could be reduced to something like the following.
+ /* 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++)
if (attnum >= 0)
- found++;
+ missing_attrs = bms_del_member(missing_attrs, attnum);
- if (found < remoterel->natts)
+ if (!bms_is_empty(missing_attrs))
+ {
+ while ((i = bms_first_memeber(missing_attrs)) >= 0)
+ {
+ if (not_first) appendStringInfo(<delimter>);
+ appendStringInfo(str, remoterel->attnames[i])
+ }
- erreport("some attrs missing");
+ ereport(ERROR, <blah blah>);
+ }
> ERROR: logical replication target relation "public.test_tbl1" is
> missing replicated columns:b1,c1,e1
I think we always double-quote identifiers in error messages. For
example:
./catalog/index.c
