Re: SQL:2011 Application Time Update & Delete

On 10.01.26 07:16, Paul A Jungwirth wrote:
> We would need to document these columns.

Done that.

> The C code uses `mltrng` a lot. Do we want to use that here? I don't
> see it in the catalog yet, but it seems clearer than `rngm`. I guess
> we have to start with `rng` though. We have `rngmultitypid`, so maybe
> `rngmulticonstr0`? Okay I understand why you went with `rngm`.

I tuned the naming again in the new patch.  I changed "constr" to 
"construct" because "constr" read too much like "constraint" to me.  I 
also did a bit of "mtlrng".  I think it's a bit more consistent and less 
ambiguous now.

> It's tempting to use two oidvectors, one for range constructors and
> another for multirange, with the 0-arg constructor in position 0,
> 1-arg in position 1, etc. We could use InvalidOid to say there is no
> such constructor. So we would have rngconstr of `{0,0,123,456}` and
> mltrngconstr of `{123,456,789}`. But is it better to avoid varlena
> columns if we can?

I don't think oidvectors would be appropriate here.  These are for when 
you have a group of values that you need together, like for function 
arguments.  But here we want to access them separately.  And it would 
create a lot of notational and a bit of storage overhead.

I had in the previous patch used some arrays as arguments in the 
internal functions, but in the second patch I'm also getting rid of that 
because it's uselessly inconsistent.

> ```
> diff --git a/src/include/catalog/pg_range.h b/src/include/catalog/pg_range.h
> index 5b4f4615905..ad4d1e9187f 100644
> --- a/src/include/catalog/pg_range.h
> +++ b/src/include/catalog/pg_range.h
> @@ -43,6 +43,15 @@ CATALOG(pg_range,3541,RangeRelationId)
>       /* subtype's btree opclass */
>       Oid            rngsubopc BKI_LOOKUP(pg_opclass);
> 
> +    /* range constructor functions */
> +    regproc        rngconstr2 BKI_LOOKUP(pg_proc);
> +    regproc        rngconstr3 BKI_LOOKUP(pg_proc);
> +
> +    /* multirange constructor functions */
> +    regproc        rngmconstr0 BKI_LOOKUP(pg_proc);
> +    regproc        rngmconstr1 BKI_LOOKUP(pg_proc);
> +    regproc        rngmconstr2 BKI_LOOKUP(pg_proc);
> +
>       /* canonicalize range, or 0 */
>       regproc        rngcanonical BKI_LOOKUP_OPT(pg_proc);
> ```
> 
> Is there a reason you're adding them in the middle of the struct? It
> doesn't help with packing.

Well, initially I had done that so that the edits to pg_range.dat are 
easier.  But I think this order makes some sense, because it has the 
mandatory data first and then the optional data later.  But it doesn't 
matter much either way.

> This needs some kind of pg_upgrade support I assume? It will have to
> work for user-defined rangetypes too.

No, I don't think there needs to be pg_upgrade support.  Existing range 
types are dumped as CREATE TYPE ... RANGE commands, and when those get 
restored it will create the new catalog entries.