pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedCREATE CAST allows creation of binary-coercible cast to range over domain
6+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2024-08-21 86488cdf12 2366ab246a Disallow creating binary-coercible casts involving range types.
* CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-20 09:00 Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2024-08-20 09:00 UTC (permalink / raw)
To: pgsql-bugs
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.)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-20 14:46 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2024-08-20 14:46 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: pgsql-bugs
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-20 17:30 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2024-08-20 17:30 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: pgsql-bugs
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
Attachments:
reject-binary-coercions-on-ranges.patch (text/x-diff, 1.9K) download
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 6593fd7d81..149642ab1b 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1689,13 +1689,18 @@ CreateCast(CreateCastStmt *stmt)
errmsg("source and target data types are not physically compatible")));
/*
- * 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.
*
* Theoretically you could build a user-defined base type that is
- * binary-compatible with a composite, enum, or array type. But we
- * disallow that too, as in practice such a cast is surely a mistake.
- * You can always work around that by writing a cast function.
+ * binary-compatible with such a type. But we disallow that too, as
+ * in practice such a cast is surely a mistake. You can always work
+ * around that by writing a cast function.
+ *
+ * NOTE: if we didn't exclude every kind of container type for this
+ * reason, we'd likely need to recursively apply all of these same
+ * checks to the contained type(s).
*/
if (sourcetyptype == TYPTYPE_COMPOSITE ||
targettyptype == TYPTYPE_COMPOSITE)
@@ -1703,6 +1708,14 @@ CreateCast(CreateCastStmt *stmt)
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("composite data types are not binary-compatible")));
+ if (sourcetyptype == TYPTYPE_RANGE ||
+ targettyptype == TYPTYPE_RANGE ||
+ sourcetyptype == TYPTYPE_MULTIRANGE ||
+ targettyptype == TYPTYPE_MULTIRANGE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("range data types are not binary-compatible")));
+
if (sourcetyptype == TYPTYPE_ENUM ||
targettyptype == TYPTYPE_ENUM)
ereport(ERROR,
* Re: CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-21 06:14 Peter Eisentraut <peter@eisentraut.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2024-08-21 06:14 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
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.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-21 07:55 Richard Guo <guofenglinux@gmail.com>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2024-08-21 07:55 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CREATE CAST allows creation of binary-coercible cast to range over domain
@ 2024-08-21 14:09 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lane @ 2024-08-21 14:09 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Peter Eisentraut <peter@eisentraut.org>; pgsql-bugs
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
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-08-21 14:09 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-08-20 09:00 CREATE CAST allows creation of binary-coercible cast to range over domain Peter Eisentraut <peter@eisentraut.org>
2024-08-20 14:46 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-08-20 17:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-08-21 06:14 ` Peter Eisentraut <peter@eisentraut.org>
2024-08-21 07:55 ` Richard Guo <guofenglinux@gmail.com>
2024-08-21 14:09 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox