Hi,
> At the time check_control_data is launched for the new cluster, the control file
> does not contain information about the set track_commit_timestamp=on. It is
> installed only in postgresql.conf.
You did consider the case that track_commit_timestamp is set to on but the instance
has not started before the pg_upgrade, right? Valid point.
> The check_track_commit_timestamp_parameter function is only needed for the new
> cluster. And you can not run it for the old cluster, but use data from the
> old_cluster.controldata.chkpnt_newstCommitTsxid control file. But this will be
> less clear than using the check_track_commit_timestamp_parameter function.
Sorry, I'm not very clear this part. Can you describe the point bit more?
Other comments:
01. get_control_data
```
+ if (GET_MAJOR_VERSION(cluster->major_version) < 905)
+ {
+ cluster->controldata.chkpnt_oldstCommitTsxid = 0;
+ cluster->controldata.chkpnt_newstCommitTsxid = 0;
+ }
```
IIUC, checksum_version is checked like this because got_data_checksum_version
must be also set.
Since we do not have the boolean for commit_ts, not sure it is needed.
old_cluster and new_cluster are the global variable and they are initialized with
zero.
02. check_track_commit_timestamp_parameter
```
+ conn = connectToServer(cluster, "template1");
+ res = executeQueryOrDie(conn, "SELECT count(*) AS is_set "
+ "FROM pg_settings WHERE name = 'track_commit_timestamp' and setting = 'on'");
+ is_set = PQfnumber(res, "is_set");
+ cluster->track_commit_timestamp_on = atoi(PQgetvalue(res, 0, is_set)) == 1;
```
The SQL looks strange for me. Isn't it enough to directly obtain "setting" attribute
and check it is "on" or not? Also, conn and res can be the local variable of the
else part.
03. ClusterInfo
```
+ bool track_commit_timestamp_on; /* track_commit_timestamp for
+ * cluster */
```
The indent is broken, please run pgindent.
04. test
Could you please update meson.build? Otherwise the added test could not be run for meson build.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
