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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to