|From:||Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>|
|To:||Nikolay Shaplov <dhyan(at)nataraj(dot)su>|
|Subject:||Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
I gave this patch a quick skim. At first I was confused by the term
"catalog"; I thought it meant we stored options in a system table. But
that's not what is meant at all; instead, what we do is build these
"catalogs" in memory. Maybe a different term could have been used, but
I'm not sure it's stricly necessary. I wouldn't really bother.
I'm confused by the "no ALTER, no lock" rule. Does it mean that if
"ALTER..SET" is forbidden? Because I noticed that brin's
pages_per_range is marked as such, but we do allow that option to change
with ALTER..SET, so there's at least one bug there, and I would be
surprised if there aren't any others.
Please make sure to mark functions as static (e.g. bringetreloptcatalog).
Why not pass ->struct_size and ->postprocess_fun (which I'd rename it as
->postprocess_fn, more in line with our naming style) as a parameter to
allocateOptionsCatalog? Also, to avoid repalloc() in most cases (and to
avoid pallocing more options that you're going to need in a bunch of
cases, perhaps that function should the number of times you expect to
call AddItems for that catalog (since you do it immediately afterwards
in all cases I can see), and allocate that number. If later another
item arrives, then repalloc using the same code you already have in
Something is wrong with leading whitespace in many places; either you
added too many tabs, or the wrong number spaces; not sure which but
visually it's clearly wrong. ... Actually there are whitespace-style
violations in several places; please fix using pgindent (after adding
any new typedefs your defining to typedefs.list).
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
|Next Message||Tom Lane||2017-03-17 17:22:39||Re: [COMMITTERS] pgsql: Use asynchronous connect API in libpqwalreceiver|
|Previous Message||Tom Lane||2017-03-17 17:18:56||Re: WIP: Faster Expression Processing v4|