RE: Patch for migration of the pg_commit_ts directory

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

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'ls7777' <ls7777@yandex.ru>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "orlovmg@gmail.com" <orlovmg@gmail.com>, "amit.kapila16@gmail.com" <amit.kapila16@gmail.com>
Date: 2025-10-09T06:22:26Z
Lists: pgsql-hackers
Hi,

Thanks for updating the patch and sorry for the late reply. Here are my comments.

01.
```
+	prep_status("Checking for pg_commit_ts");
```

I think we must clarify which node is being checked. Something like:
Checking for new cluster configuration for commit timestamp

02.
```
	}
-
 	/* we have the result of cmd in "output". so parse it line by line now */
```

This change is not needed.

03.
```
+	/*
+	 * Copy pg_commit_ts
+	 */
```

I feel it can be removed or have more meanings. Something lile:
Copy old commit_timestamp data to new, if available.

04.

Regarding the test, 


05.
```
+sub command_output
```

Can run_command() be used instead of defining new function?

06.
```
+$old->command_ok([ 'pgbench', '-i', '-s', '1' ], 'init pgbench');
+$old->command_ok([ 'pgbench', '-t', '1', '-j', '1' ], 'pgbench it');
```

I think no need to use pgbench anymore. Can we define tables and insert tuples
by myself?

07.
```
+command_fails(
+	[
+		'pg_upgrade', '--no-sync',
+		'-d', $old->data_dir,
+		'-D', $new->data_dir,
+		'-b', $old->config_data('--bindir'),
+		'-B', $new->config_data('--bindir'),
+		'-s', $new->host,
+		'-p', $old->port,
+		'-P', $new->port,
+		$mode
+	],
+	'run of pg_upgrade fails with mismatch parameter track_commit_timestamp');
```

According to other test files, we do not use the shorten option.
Also, please verify the actual output by command_ok_or_fails_like() or command_checks_all().

08.
```
+sub xact_commit_ts
```

Not sure, but this function is introduced because we have 100 transactions. Can we omit this now?

09.
```
+# The new cluster should contain timestamps created during the pg_upgrade and
+# some more created by the pgbench.
+#
+print "\nCommit timestamp only in new cluster:\n";
+for my $line (@b) {
+	print "$line\n" unless $h1{$line};
+}
```

I don't think this is needed because the output is not verified.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

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