RE: Patch for migration of the pg_commit_ts directory
От | Hayato Kuroda (Fujitsu) |
---|---|
Тема | RE: Patch for migration of the pg_commit_ts directory |
Дата | |
Msg-id | OSCPR01MB14966709F525419547BA3F99DF5E0A@OSCPR01MB14966.jpnprd01.prod.outlook.com обсуждение исходный текст |
Ответ на | RE: Patch for migration of the pg_commit_ts directory ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>) |
Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: