Horton
Menu

RE: Pgoutput not capturing the generated columns

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Shubham Khanna' <khannashubham1197@gmail.com>
Cc: Shlok Kyal <shlok.kyal.oss@gmail.com>, vignesh C <vignesh21@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "euler@eulerto.com" <euler@eulerto.com>
Date: 2024-06-23T04:58:15Z
Lists: pgsql-hackers 
Hi Shubham,

Thanks for sharing new patch! You shared as v9, but it should be v10, right?
Also, since there are no commitfest entry, I registered [1]. You can rename the
title based on the needs. Currently CFbot said OK.

Anyway, below are my comments.

01. General
Your patch contains unnecessary changes. Please remove all of them. E.g., 

```
 						 " s.subpublications,\n");
-
```
And
```
 		appendPQExpBufferStr(query, " o.remote_lsn AS suboriginremotelsn,\n"
-							 " s.subenabled,\n");
+							" s.subenabled,\n");
```

02. General
Again, please run the pgindent/pgperltidy.

03. test_decoding
Previously I suggested to the default value of to be include_generated_columns
should be true, so you modified like that. However, Peter suggested opposite
opinion [3] and you just revised accordingly. I think either way might be okay, but
at least you must clarify the reason why you preferred to set default to false and
changed accordingly.

04. decoding_into_rel.sql
According to the comment atop this file, this test should insert result to a table.
But added case does not - we should put them at another place. I.e., create another
file.

05. decoding_into_rel.sql
```
+-- when 'include-generated-columns' is not set
```
Can you clarify the expected behavior as a comment?

06. getSubscriptions
```
+	else
+		appendPQExpBufferStr(query,
+						" false AS subincludegencols,\n");
```
I think the comma is not needed.
Also, this error meant that you did not test to use pg_dump for instances prior PG16.
Please verify whether we can dump subscriptions and restore them accordingly.

[1]: https://commitfest.postgresql.org/48/5068/
[2]: https://www.postgresql.org/message-id/OSBPR01MB25529997E012DEABA8E15A02F5E52%40OSBPR01MB2552.jpnprd01.prod.outlook.com
[3]: https://www.postgresql.org/message-id/CAHut%2BPujrRQ63ju8P41tBkdjkQb4X9uEdLK_Wkauxum1MVUdfA%40mail.gmail.com

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/

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.