Re: WAL replay bugs

Hello,

> Thanks for your comments. Looking forward to seeing some more input.

Thank you. bufcapt.c was a poser.

bufcapt.c: 326 memcpy(&tail, &page[BLCKSZ - 2], 2);

This seems duzzling.. Isn't "*(uint16*)(&page[BLCKSZ - 2])" applicable?

bufcapt.c: 331 else if (PageGetSpecial....

Generally saying, the code to identify the type of page is too
heuristic and seems fragile.

Pehaps this is showing that no tidy or generalized way to tell
what a page is used for. Many of the modules which have their
own page format has a magic value and the values seem to be
selected carefully. But no one-for-all method to retrieve that.

Each type of page can be confirmed by the following way *if*
its type is previously *hinted* except for gin.

btree : 32bit magic at pd->opaque
gin : No magic
gist : 16-bit magic at ((GISTPageOpaque*)pd->opaque)->gist_page_id
spgist : 16-bit magic at ((SpGistPageOpaque*)pd->opaque)->spgist_page_id
hash : 16-bit magic at ((HashPageOpaque*)pd->paque)->hasho_page_id
sequence : 16-bit magic at pd->opaque, the last 2 bytes of the page.

# Is it comprehensive? and correct?

The majority is 16-bit magic at the TAIL of opaque struct. If
we can unify them into , say, 16-bit magic at
*(uint16*)(pd->opaque) the sizeof() labyrinth become stable and
simple and other tools which should identify the type of pages
will be happy. Do you think we can do that?

# Sorry, time's up for today.

> > - contrib/buffer_capture_cmp/README
..
> Yeah right... This was a rest of some previous hacking on this feature.
> Paragraph was rather unclear so I rewrote it, mentioning that the custom
> script can use PGPORT to connect to the node where tests can be run.
>
> - contrib/buffer_capture_cmp/Makefile
> >
> > "make check" does nothing when BUFFER_CAPTURE is not defined, as
...
> Sure, I added such a message in the makefile.
>
> - buffer_capture_cmp.c
> >
> > This source generates void executable when BUFFER_CAPTURE is
..
> Done. The compilation of this utility is now independent on BUFFER_CAPTURE.
> At the same time I made test.sh a bit smarter to have it grab the value of
> BUFFER_CAPTURE_FILE directly from bufcapt.h.
>
> - buffer_capture_cmp.c/main()
> >
> > The parameters for this command are the parent directories for
...
> Fixed. I changed back the utility to directly file names instead of data
> folders as arguments.
>
> Updated patches addressing those comments are attached.
> Regards,

Thank you I'll look into them later.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center