Re: Range Types - typo + NULL string constructor
От | Heikki Linnakangas |
---|---|
Тема | Re: Range Types - typo + NULL string constructor |
Дата | |
Msg-id | 4E98BBC2.60406@enterprisedb.com обсуждение исходный текст |
Ответ на | Re: Range Types - typo + NULL string constructor (Jeff Davis <pgsql@j-davis.com>) |
Ответы |
Re: Range Types - typo + NULL string constructor
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: Range Types - typo + NULL string constructor (Jeff Davis <pgsql@j-davis.com>) |
Список | pgsql-hackers |
On 07.10.2011 06:41, Jeff Davis wrote: > The repo is available here: > > http://git.postgresql.org/gitweb/?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes I took a look at this, fixed a bunch of minor issues, and pushed to my git repo: git://git.postgresql.org/git/users/heikki/postgres.git. Attached is an updated patch including those changes, for the archives. A few issues that I didn't fix straight away: * Why do you need to be a superuser to create a range type? In DefineRange, the comment explaining why superuser privilege is required seems bogus, copy-pasted from base types. So what is the reason? * The binary i/o format includes the length of the lower and upper bounds twice, once explicitly in range_send, and second time within the send-function of the subtype. Seems wasteful. * In findRangeSubOpclass, I think it's OK to hard-code "btree" into the error message, no need to look that up from pg_am. * Do we really need non_empty(anyrange) ? You can just do "NOT empty(x)" * Range_before and range_after: I think "input range is empty" would sound better than "undefined for empty ranges". * In range_hash, rotating the hash of the initial flags byte seems pointless * range_constructor_internal - I think it would be better to move logic to figure out the the arguments into the callers. * The gist support functions frequently call range_deserialize(), which does catalog lookups. Isn't that horrendously expensive? * In range_serialize and range_deserialize, reduce the number of catalog lookups by combining the get_range_subtype, get_typlen, get_typalign, get_typbyval and get_typstorage calls with just one catalog lookup, and fetch all those fields at once. Something like get_typlenbyvalalign() * Have you tested this on an architecture with strict alignment? I don't see any alignment bugs, but I think there's a lot of potential for them.. Documentation ------------- * Section "Built-in Range Types". How about "PostgreSQL comes with the following built-in range types:" <examples> "In addition, you can define your own" * In section "Inclusive and Exclusive Bounds", it is said "An inclusive lower bound is represented by ...". That begs the question: where is it represented like that? That doesn't become clear until you read the section on Input/Output format. Reordering the sections might help. Are the double-quotes around [ and ( necessary? * It would be good to have some examples on the input formats in the Input/Output section. There's one in the Examples section, but it's not very clear that it's related. Perhaps the whole Examples section should be moved to the end, or the examples be dispersed into the other sections. * Likewise it would be nice to have some examples in the Inclusive and Exclusive Bounds section. * subtype_float, how is it supposed to work? I couldn't find any explanation in the docs. Can we come up with a better name for the function? * Constructing Ranges section: should describe the 0-3 argument forms of the constructors, not only in the examples but in the main text too. * What exactly is canonical function supposed to return? It's not clear what format one should choose as the canonical format when writing a custom range type. And even for the built-in types, it would be good to explain which types use which canonical format (I saw the discussions on that, so I understand that might still be subject to change). * Defining New RangeTypes section: A more general description in addition to the example would be good. * "GiST Indexing" title: how about making it just "Indexing". Also, it would be nice to list exactly which operators can be sped up by gist. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Вложения
В списке pgsql-hackers по дате отправления: