The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation: tested, failed
Hello, I tested this patch on Linux and there is no problem. Also, I reviewed this patch and commented below. @@ -439,6 +447,107 @@ XLogRecordHasFPW(XLogReaderState *record) + if (fork >= 0 && fork <= MAX_FORKNUM) + { + if (fork) + sprintf(forkname, "_%s", forkNames[fork]); + else + forkname[0] = 0; + } + else + pg_fatal("found invalid fork number: %u", fork); Should we add the comment if the main fork is saved without "_main" suffix for code readability? @@ -679,6 +788,9 @@ usage(void) " (default: 1 or the value used in STARTSEG)\n")); printf(_(" -V, --version output version information, then exit\n")); printf(_(" -w, --fullpage only show records with a full page write\n")); + printf(_(" -W, --save-fpi=path save full page images to given path as\n" + " LSN.T.D.R.B_F\n")); + printf(_(" -X, --fixup-fpi=path like --save-fpi but apply LSN fixups to saved page\n")); printf(_(" -x, --xid=XID only show records with transaction ID XID\n")); printf(_(" -z, --stats[=record] show statistics instead of records\n" " (optionally, show per-record statistics)\n")); Since lower-case options are displayed at the top, should we switch the order of -x and -X? @@ -972,6 +1093,25 @@ main(int argc, char **argv) } } + int dir_status = pg_check_dir(config.save_fpw_path); + + if (dir_status < 0) + { + pg_log_error("could not access output directory: %s", config.save_fpw_path); + goto bad_argument; + } Should we output %s enclosed with \"? Regards, Sho Kato