Maxim,

I see both of us accounted for Alexanders feedback and submitted v59.
Your newer version seems to have issues on cfbot, so resubmitting the
previous patchset that passes the tests. Please feel free to add
changes.

> See SlruCorrectSegmentFilenameLength():
>
> ```
>     if (ctl->long_segment_names)
>         return (len == 15); /* see SlruFileName() */
>     else
>         /*
>          * Commit 638cf09e76d allowed 5-character lengths. Later commit
>          * 73c986adde5 allowed 6-character length.
>          *
>          * XXX should we still consider such names to be valid?
>          */
>         return (len == 4 || len == 5 || len == 6);
> ```
>
> Should we just drop it or check that segno is <= 0xFFFFFF?

I also choose to change this Assert and to add a corresponding comment:

        else
        {
-               Assert(segno >= 0 && segno <= 0xFFFF);
+               /*
+                * Despite the fact that %04X format string is used up to 24 bit
+                * integers are allowed. See SlruCorrectSegmentFilenameLength()
+                */
+               Assert(segno >= 0 && segno <= 0xFFFFFF);
                return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
                                                (unsigned int) segno);
        }


--
Best regards,
Aleksander Alekseev

Attachment: v60-0001-Index-SLRUs-by-64-bit-integers-rather-than-by-32.patch
Description: Binary data

Attachment: v60-0003-Make-use-FullTransactionId-in-2PC-filenames.patch
Description: Binary data

Attachment: v60-0002-Use-larger-segment-file-names-for-pg_notify.patch
Description: Binary data

Reply via email to