RE: Improve the error message for logical replication of regular column to generated column.
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Shubham Khanna' <khannashubham1197@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-11-15T02:49:16Z
Lists: pgsql-hackers
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.gmail.com Best regards, Hayato Kuroda FUJITSU LIMITED
Commits
-
Fix buildfarm failure from commit 8fcd80258b.
- 9d7aa406d04c 18.0 landed
-
Improve error message for replication of generated columns.
- 8fcd80258bcf 18.0 landed