From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | Joe Conway <mail(at)joeconway(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, amul sul <sulamul(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Multi column range partition table |
Date: | 2017-07-12 09:46:13 |
Message-ID: | CAFjFpRd1QbXeDA7B96Z+exdMkmHp4JecyP_S_qka_HJudsWppw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 12, 2017 at 12:54 AM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> On 11 July 2017 at 13:29, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> + <para>
>> + Also note that some element types, such as <literal>timestamp</>,
>> + have a notion of "infinity", which is just another value that can
>> + be stored. This is different from <literal>MINVALUE</> and
>> + <literal>MAXVALUE</>, which are not real values that can be stored,
>> + but rather they are ways of saying the value is unbounded.
>> + <literal>MAXVALUE</> can be thought of as being greater than any
>> + other value, including "infinity" and <literal>MINVALUE</> as being
>> + less than any other value, including "minus infinity". Thus the range
>> + <literal>FROM ('infinity') TO (MAXVALUE)</> is not an empty range; it
>> + allows precisely one value to be stored — the timestamp
>> + "infinity".
>> </para>
>>
>> The description in this paragraph seems to be attaching intuitive
>> meaning of word "unbounded" to MAXVALUE and MINVALUE, which have
>> different intuitive meanings of themselves. Not sure if that's how we
>> should describe MAXVALUE/MINVALUE.
>>
>
> I'm not sure I understand your point. MINVALUE and MAXVALUE do mean
> unbounded below and above respectively. This paragraph is just making
> the point that that isn't the same as -/+ infinity.
>
What confuses me and probably users is something named min/max"value"
is not a value but something lesser or greater than any other "value".
The paragraph above explains that <literal>FROM ('infinity') TO
(MAXVALUE)</> implies a partition with only infinity value in there.
What would be the meaning of <literal>FROM (MINVALUE) TO ('minus
infinity')</>, would that be allowed? What would it contain esp. when
the upper bounds are always exclusive?
>
>> Most of the patch seems to be replacing "content" with "kind",
>> RangeDatumContent with PartitionRangeDatumKind and RANGE_DATUM_FINITE
>> with PARTITION_RANGE_DATUM_VALUE. But those changes in name don't seem
>> to be adding much value to the patch. Replacing RANGE_DATUM_NEG_INF
>> and RANGE_DATUM_POS_INF with PARTITION_RANGE_DATUM_MINVALUE and
>> PARTITION_RANGE_DATUM_MAXVALUE looks like a good change in line with
>> MINVALUE/MAXVALUE change. May be we should reuse the previous
>> variables, enum type name and except those two, so that the total
>> change introduced by the patch is minimal.
>>
>
> No, this isn't just renaming that other enum. It's about replacing the
> boolean "infinite" flag on PartitionRangeDatum with something that can
> properly enumerate the 3 kinds of PartitionRangeDatum that are allowed
> (and, as noted above "finite"/"infinite isn't the right terminology
> either).
Right. I think we need that change.
> Putting that new enum in parsenodes.h makes it globally
> available, wherever the PartitionRangeDatum structure is used. A
> side-effect of that change is that the old RangeDatumContent enum that
> was local to partition.c is no longer needed.
Hmm, I failed to notice the changes in _out, _equal, _read functions.
The downside is that enum can not be used for anything other than
partitioning. But I can not imagine where will we use it though.
>
> RangeDatumContent wouldn't be a good name for a globally visible enum
> of this kind because that name fails to link it to partitioning in any
> way, and could easily be confused as having something to do with RTEs
> or range types. Also, the term "content" is more traditionally used in
> the Postgres sources for a field *holding* content, rather than a
> field specifying the *kind* of content. On the other hand, you'll note
> that the term "kind" is by far the most commonly used term for naming
> this kind of enum, and any matching fields.
Ok.
>
> IMO, code consistency and readability takes precedence over keeping
> patch sizes down.
No doubt about that.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2017-07-12 09:47:01 | Re: WIP Patch: Pgbench Serialization and deadlock errors |
Previous Message | Heikki Linnakangas | 2017-07-12 09:32:58 | Re: Minor style cleanup in tab-complete.c |