Horton

Thread

Commits

  1. Disallow creating binary-coercible casts involving range types.

  1. CREATE CAST allows creation of binary-coercible cast to range over domain

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-20T09:00:21Z

    CREATE CAST disallows creating a binary-coercible cast to a domain 
(because that would bypass checking the domain constraints).  But it 
allows it if the domain is wrapped inside a range type:

CREATE DOMAIN mydomain AS int4 CHECK (VALUE > 0);
CREATE CAST (int4 AS mydomain) WITHOUT FUNCTION;  -- error (ok)

CREATE TYPE mydomainrange AS range (subtype=mydomain);
CREATE CAST (int4range AS mydomainrange) WITHOUT FUNCTION;  -- FIXME

SELECT int4range(-5,-4)::mydomainrange;  -- this succeeds

This particular case seems straightforward to fix, but maybe there are 
also cases with more nesting to consider.

(I just found this while exploring other range-over-domain issues in 
some in-progress work.)

  2. Re: CREATE CAST allows creation of binary-coercible cast to range over domain

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-08-20T14:46:15Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> CREATE CAST disallows creating a binary-coercible cast to a domain 
> (because that would bypass checking the domain constraints).  But it 
> allows it if the domain is wrapped inside a range type:
> ...
> This particular case seems straightforward to fix, but maybe there are 
> also cases with more nesting to consider.

I think it's an oversight that we allow binary-coercible casts
involving range types at all.  There are no other nesting cases to
worry about, because CreateCast already rejects binary-coercible casts
applied to composites and arrays, explaining

         * We know that composite, enum and array types are never binary-
         * compatible with each other.  They all have OIDs embedded in them.

But range types also embed their own OID, so I don't see why that
concern doesn't apply to them.

			regards, tom lane

  3. Re: CREATE CAST allows creation of binary-coercible cast to range over domain

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-08-20T17:30:39Z

    I wrote:
> I think it's an oversight that we allow binary-coercible casts
> involving range types at all.  There are no other nesting cases to
> worry about, because CreateCast already rejects binary-coercible casts
> applied to composites and arrays, explaining
>          * We know that composite, enum and array types are never binary-
>          * compatible with each other.  They all have OIDs embedded in them.
> But range types also embed their own OID, so I don't see why that
> concern doesn't apply to them.

In short, more or less as attached.  (I didn't bother with a
regression test, since none of the adjacent error checks are
covered either.)

I'm unsure whether to back-patch this.  If somebody were using
such a cast today, it might seem to mostly work, as long as the
two kinds of ranges had the same behavior.  So they might not
appreciate us breaking it in a minor release.  Maybe we should
put this into 17 but not further back.

			regards, tom lane

  4. Re: CREATE CAST allows creation of binary-coercible cast to range over domain

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-21T06:14:23Z

    On 20.08.24 19:30, Tom Lane wrote:
> I wrote:
>> I think it's an oversight that we allow binary-coercible casts
>> involving range types at all.  There are no other nesting cases to
>> worry about, because CreateCast already rejects binary-coercible casts
>> applied to composites and arrays, explaining
>>           * We know that composite, enum and array types are never binary-
>>           * compatible with each other.  They all have OIDs embedded in them.
>> But range types also embed their own OID, so I don't see why that
>> concern doesn't apply to them.
> 
> In short, more or less as attached.  (I didn't bother with a
> regression test, since none of the adjacent error checks are
> covered either.)

This patch looks right.

> I'm unsure whether to back-patch this.  If somebody were using
> such a cast today, it might seem to mostly work, as long as the
> two kinds of ranges had the same behavior.  So they might not
> appreciate us breaking it in a minor release.  Maybe we should
> put this into 17 but not further back.

Yeah, 17 and up seems ok to me.

  5. Re: CREATE CAST allows creation of binary-coercible cast to range over domain

    Richard Guo <guofenglinux@gmail.com> — 2024-08-21T07:55:30Z

    On Wed, Aug 21, 2024 at 2:14 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> On 20.08.24 19:30, Tom Lane wrote:
> > In short, more or less as attached.  (I didn't bother with a
> > regression test, since none of the adjacent error checks are
> > covered either.)
>
> This patch looks right.

This patch also looks good to me.  To nitpick:

-    * We know that composite, enum and array types are never binary-
-    * compatible with each other.  They all have OIDs embedded in them.
+    * We know that composite, array, range and enum types are never
+    * binary-compatible with each other.  They all have OIDs embedded in
+    * them.

I wonder if it would be better for readability to list these types in
the order we check them in the code, as we did previously, i.e.:

    * We know that composite, range, enum and array types are never
    * ...

Thanks
Richard

  6. Re: CREATE CAST allows creation of binary-coercible cast to range over domain

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-08-21T14:09:58Z

    Richard Guo <guofenglinux@gmail.com> writes:
> I wonder if it would be better for readability to list these types in
> the order we check them in the code, as we did previously, i.e.:

The previous order seemed quite random to me, because enums aren't
container types and the reason for excluding them isn't really the
same.  I thought about re-ordering the code to match the new comment,
but desisted.  We could do that though...

			regards, tom lane