public inbox for [email protected]  

help / color / mirror / Atom feed

From: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Shubham Khanna' <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Improve the error message for logical replication of regular column to generated column.
Date: Fri, 15 Nov 2024 02:49:16 +0000
Message-ID: <TYAPR01MB569230B7BF79C08284FEB706F5242@TYAPR01MB5692.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAHv8RjJBvYtqU7OAofBizOmQOK2Q8h+w9v2_cQWxT_gO7er3Aw@mail.gmail.com>
References: <CAHv8RjJBvYtqU7OAofBizOmQOK2Q8h+w9v2_cQWxT_gO7er3Aw@mail.gmail.com>

Dear Shubham,

Thanks for creating a patch! I checked yours and I have comments.

01.
```
+		StringInfoData gencolsattsbuf;
+		int			generatedatts = 0;
+
+		initStringInfo(&gencolsattsbuf);
```

gencolsattsbuf is initialized at the beginning but won't be free'd.

But I prefer the Peter's suggestion - you can combine the reporting stuff to
logicalrep_report_missing_attrs and rename the function. This is clearer than
directly adding declarations and ereport() in logicalrep_rel_open().

02.

```
+					/*
+					 * Check if the subscription table generated column has
+					 * same name as a non-generated column in the
+					 * corresponding publication table.
+					 */
```

I don't think this comment is correct. The error can be reported even when
both publisher and subscriber has the generated column, right?
Also, I feel comments can be located atop "if".

03.
I feel if you combine the reporting stuff with logicalrep_report_missing_attrs, some
of changes are not needed anymore. You can just add comment in logicalrep_rel_open
and modify the message in logicalrep_report_missing_attrs.


[1]: https://www.postgresql.org/message-id/CAHut%2BPumbPEqk6v2XVjT7vKWKzQNBjMHXByWJ5%3DFmjEfk1v_pQ%40mail...

Best regards,
Hayato Kuroda
FUJITSU LIMITED

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: RE: Improve the error message for logical replication of regular column to generated column.
  In-Reply-To: <TYAPR01MB569230B7BF79C08284FEB706F5242@TYAPR01MB5692.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox