Re: Sequence Access Method WIP
От | Jose Luis Tallon |
---|---|
Тема | Re: Sequence Access Method WIP |
Дата | |
Msg-id | 20160330132241.1315.90869.pgcf@coridan.postgresql.org обсуждение исходный текст |
Ответ на | Re: Sequence Access Method WIP (Fabrízio de Royes Mello <fabriziomello@gmail.com>) |
Ответы |
Re: Sequence Access Method WIP
|
Список | pgsql-hackers |
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation: not tested [Partial review] Evaluated: 0002-gapless-seq-2016-03-29-2.patch Needs updating code copyright years ... or is this really from 2013? [ contrib/gapless_seq/gapless_seq.c ] Patch applies cleanly to current master (3063e7a84026ced2aadd2262f75eebbe6240f85b) It does compile cleanly. DESIGN The decision to hardcode the schema GAPLESS_SEQ_NAMESPACE ("gapless_seq") and VALUES_TABLE_NAME ("seqam_gapless_values")strikes me a bit: while I understand the creation of a private schema named after the extension,it seems overkill for just a single table. I would suggest to devote some reserved schema name for internal implementation details and/or AM implementation details,if deemed reasonable. On the other hand, creating the schema/table upon extension installation makes the values table use the default tablespacefor the database, which can be good for concurrency --- direct writes to less loaded storage (Note that usersmay want to move this table into an SSD-backed tablespace or equivalently fast storage ... moreso when many --not justone-- gapless sequences are being used) Yet, there is <20141203102425.GT2456@alap3.anarazel.de> where Andres argues against anything different than one-page-per-sequenceimplementations. I guess this patch changes everything in this respect. CODE seqam_gapless_init(Relation seqrel, Form_pg_sequence seq, int64 restart_value, bool restart_requested,bool is_init) -> is_init sounds confusing; suggest renaming to "initialize" or "initial" to avoid readingas "is_initIALIZED" DEPENDS ON 0001-seqam-v10.patch , which isn't commited yet --- and it doesn't apply cleanly to current git master. Please update/rebase the patch and resubmit.
В списке pgsql-hackers по дате отправления: