Horton
Menu

Re: Pgoutput not capturing the generated columns

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Shubham Khanna <khannashubham1197@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>, pgsql-hackers@lists.postgresql.org, euler@eulerto.com
Date: 2024-09-24T01:37:49Z
Lists: pgsql-hackers

Attachments

Hi. Here are my v32-0002 review comments:

======
src/backend/replication/logical/tablesync.c

1. fetch_remote_table_info

  /*
- * Get column lists for each relation.
+ * Get column lists for each relation, and check if any of the
+ * publications have the 'publish_generated_columns' parameter enabled.

I am not 100% sure about this logic anymore. Maybe it is OK, but it
requires careful testing because with Amit's "column lists take
precedence" it is now possible for the publication to say
'publish_generated_columns=false', but the publication can still
publish gencols *anyway* if they were specified in a column list.

~~~

2.
  /*
  * Fetch info about column lists for the relation (from all the
  * publications).
  */
+ StringInfo pub_names = makeStringInfo();
+
+ get_publications_str(MySubscription->publications, pub_names, true);
  resetStringInfo(&cmd);
  appendStringInfo(&cmd,
~

nit - The comment here seems misplaced.

~~~

3.
+ if (server_version >= 120000)
+ {
+ has_pub_with_pubgencols = server_version >= 180000 && has_pub_with_pubgencols;
+
+ if (!has_pub_with_pubgencols)
+ appendStringInfo(&cmd, " AND a.attgenerated = ''");
+ }

My previous review comment about this [1 #10] was:
Can the 'gencols_allowed' var be removed, and the condition just be
replaced with if (!has_pub_with_pubgencols)? It seems equivalent
unless I am mistaken.

nit - So the current v32 code is not what I was expecting. What I
meant was 'has_pub_with_pubgencols' can only be true if server_version
>= 180000, so I thought there was no reason to check it again. For
reference, I've changed it to like I meant in the nitpicks attachment.
Please see if that works the same.

======
[1] my review of v31-0002.
https://www.postgresql.org/message-id/CAHut%2BPusbhvPrL1uN1TKY%3DFd4zu3h63eDebZvsF%3Duy%2BLBKTwgA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Generated column replication.

  2. Rename pubgencols_type to pubgencols in pg_publication.

  3. Doc: Fix column name in pg_publication catalog.

  4. Fix buildfarm failure introduced by commit e65dbc9927.

  5. Change publication's publish_generated_columns option type to enum.

  6. Fix \dRp+ output when describing publications with a lower server version.

  7. Replicate generated columns when 'publish_generated_columns' is set.

  8. Doc: Update the behavior of generated columns in Logical Replication.

  9. Replicate generated columns when specified in the column list.

  10. Doc: Generated columns are skipped for logical replication.