Anastasia Lubennikova wrote:
> Working on page compression and some other issues related to
> access methods, I found out that the code related to heap
> looks too complicated. Much more complicated, than it should be.
> Since I anyway got into this area, I want to suggest a set of improvements.
Hm. I'm hacking on heapam.c pretty heavily ATM. Your first patch
causes no problem I think, or if it does it's probably pretty minor, so
that's okay. I'm unsure about the second one -- I think it's okay too,
because it mostly seems to be about moving stuff from heapam.c to hio.c
and shuffling some code around that I don't think I'm modifying.
> Also I think that it's quite strange to have a function heap_create(), that
> is called
> from index_create(). It has absolutely nothing to do with heap access
> method.
Agreed. But changing its name while keeping it in heapam.c does not
really improve things enough. I'd rather have it moved elsewhere that's
not specific to "heaps" (somewhere in access/common perhaps). However,
renaming the functions would break third-party code for no good reason.
I propose that we only rename things if we also break it for other
reasons (say because the API changes in a fundamental way).
> One more thing that requires refactoring is "RELKIND_RELATION" name.
> We have a type of relation called "relation"...
I don't see us fixing this bit, really. Historical baggage and all
that. For example, a lot of SQL queries use "WHERE relkind = 'r'". If
we change the name, we should probably also change the relkind constant,
and that would break the queries. If we change the name and do not
change the constant, then we have to have a comment "we call them
RELKIND_TABLE but the char is r because it was called RELATION
previously", which isn't any better.
> So there is a couple of patches. They do not cover all mentioned problems,
> but I'd like to get a feedback before continuing.
I agree that we could improve things in this general area, but I do not
endorse any particular changes you propose in these patches; I haven't
reviewed your patches.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services