public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kirill Reshke <[email protected]>
To: Peter Eisentraut <[email protected]>
Cc: Paul A Jungwirth <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 Application Time Update & Delete
Date: Mon, 19 Jan 2026 22:43:44 +0500
Message-ID: <CALdSSPirmOR9veEcB7Y0PXbH=CE3a_+vdsvO74_sGeT1VW6tFQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CA+renyUiEu2yTHk+Vy-Rt2xA5Vnp2N_ohN=npiNmMKTh53qDfA@mail.gmail.com>
<[email protected]>
<CA+renyW7ZB_k9AgmSFJU2EegL9r1k1sgWo4-9tGGkgwxNqe6kw@mail.gmail.com>
<CA+renyUodzxAvMnpa_LTvo+Ru1ZKH+Su8KaPvD4iMtguFKzq4g@mail.gmail.com>
<[email protected]>
<CA+renyU-iz_zvM0gGP=dvBPVrz=Jj3qdCjtAh5nLZRhb49xMFw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CA+renyXchHgpMbYX-cR8fuNnnpf_+FJ6PkHoXoa2AgzRnz4vxQ@mail.gmail.com>
<[email protected]>
<CA+renyXH3AF6JVzZGVcT5mAo=0QncB-MpWJeqb2JG66sgyq09g@mail.gmail.com>
<[email protected]>
<CA+renyUazgR-hB_6RY60n23L0y-n_h9G1AappZmPENO0k5pL1g@mail.gmail.com>
<[email protected]>
<CA+renyVXg5pV84wQnGQuK8-=qoKw3BiBgQzesxM_LkcxxWmYjA@mail.gmail.com>
<[email protected]>
On Mon, 19 Jan 2026 at 18:37, Peter Eisentraut <[email protected]> wrote:
>
> 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.
Hi!
I have looked into v2. This patch looks good. Making explicit links in
pg_catalog seems to be more cve-proof to me. Using Paul's approach
(get_typname_and_namespace) is not only fragile, it is a recipe for
CVE if any mistake is made, is it? I mean, matching something by name
is vulnerable for search-path-based CVE (again, not saying this is the
case in Paul patch).
I think patch tests are good. Also, I don't think we need to mention
any "upcoming patches" in the commit message - this change has its own
value.
One stupid question from me: should we add
````
t.typanalyze!='range_typanalyze'::regproc or t.typinput !=
'range_in'::regproc or t.typoutput != 'range_out'::regproc or
t.typreceive != 'range_recv'::regproc or typsend !=
'range_send'::regproc;
````
In type sanity sql check? In my understanding, this condition
(t.typanalyze == 'range_typanalyze'::regproc and ....) is required
for built-in range types, and for user-defined seems to also be true.
--
Best regards,
Kirill Reshke
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: SQL:2011 Application Time Update & Delete
In-Reply-To: <CALdSSPirmOR9veEcB7Y0PXbH=CE3a_+vdsvO74_sGeT1VW6tFQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox