Horton

Thread

Commits

  1. Create a new type category for "internal use" types.

  1. Dubious usage of TYPCATEGORY_STRING

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-02T21:22:21Z

    The parser's type-coercion heuristics include some special rules
for types belonging to the STRING category, which are predicated
on the assumption that such types are reasonably general-purpose
string types.  This assumption has been violated by a few types,
one error being ancient and the rest not so much:

1. The "char" type is labeled as STRING category, even though it's
(a) deprecated for general use and (b) unable to store more than
one byte, making "string" quite a misnomer.

2. Various types we invented to store special catalog data, such as
pg_node_tree and pg_ndistinct, are also labeled as STRING category.
This seems like a fairly bad idea too.

An example of the reasons not to treat these types as being
general-purpose strings can be seen at [1], where the "char"
type has acquired some never-intended cast behaviors.  Taking
that to an extreme, we currently accept

regression=# select '(1,2)'::point::"char";
 char 
------
 (
(1 row)

My first thought about fixing point 1 was to put "char" into some
other typcategory, but that turns out to break some of psql's
catalog queries, with results like:

regression=# \dp
ERROR:  operator is not unique: unknown || "char"
LINE 16:            E' (' || polcmd || E'):'
                          ^
HINT:  Could not choose a best candidate operator. You might need to add explicit type casts.

I looked briefly at rejiggering the casting rules so that that
would still work, but it looks like a mess.  The problem is that
unknown || "char" can match either text || text or text || anynonarray,
and it's only the special preference for preferred types *of the
same typcategory as the input type* that allows us to prefer one
of those over the other.

Hence, what 0001 below does is to leave "char" in the string
category, but explicitly disable its access to the special
cast-via-I/O rules.  This is a hack for sure, but it won't have
any surprising side-effects on other types, which changing the
general operator-matching rules could.  The only thing it breaks
in check-world is that contrib/citext expects casting between
"char" and citext to work.  I think that's not a very reasonable
expectation so I just took out the relevant test cases.  (If anyone
is hot about it, we could add explicit support for such casts in
the citext module ... but it doesn't seem worth the trouble.)

As for point 2, I haven't found any negative side-effects of
taking the special types out of the string category, so 0002
attached invents a separate TYPCATEGORY_INTERNAL category to
put them in.

Thoughts?

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/CAOC8YUcXymCMpC5d%3D7JvcwyjXPTT00WeebOM3UqTBreOD1N9hw%40mail.gmail.com

  2. types reliant on encodings [was Re: Dubious usage of TYPCATEGORY_STRING]

    Chapman Flack <chap@anastigmatix.net> — 2021-12-03T18:42:30Z

    On 12/02/21 16:22, Tom Lane wrote:
> ... types belonging to the STRING category, which are predicated
> on the assumption that such types are reasonably general-purpose
> string types.

This prods me to submit a question I've been incubating for a while.

Is there any way to find out, from the catalogs or in any automatable way,
which types are implemented with a dependence on the database encoding
(or on some encoding)?

You might think S category types, for a start: name, text, character,
varchar, all dependent on the server encoding, as you'd expect. The ones
Tom moves here to category Z were most of the ones I wondered about.

Then there's "char". It's category S, but does not apply the server
encoding. You could call it an 8-bit int type, but it's typically used
as a character, making it well-defined for ASCII values and not so
for others, just like SQL_ASCII encoding. You could as well say that
the "char" type has a defined encoding of SQL_ASCII at all times,
regardless of the database encoding.

U types are a mixed bag. Category U includes bytea (no character encoding)
and xml/json/jsonb (server encoding). Also tied to the server encoding
are cstring and unknown.

As an aside, I think it's unfortunate that the xml type has this implicit
dependency on the server encoding, when XML is by definition Unicode.
It means there are valid XML documents that PostgreSQL may not be able
to store, and which documents those are depends on what the database
encoding is. I think json and jsonb suffer in the same way.

Changing that would be disruptive at this point and I'm not suggesting it,
but there might be value in the thought experiment to see what the
alternate universe would look like.

In the alternate world, you would know that certain datatypes were
inherently encoding-oblivious (numbers, polygons, times, ...), certain
others are bound to the server encoding (text, varchar, name, ...), and
still others are bound to a known encoding other than the server encoding:
the ISO SQL NCHAR type (bound to an alternate configurable database
encoding), "char" (always SQL_ASCII), xml/json/jsonb (always with the full
Unicode repertoire, however they choose to represent it internally).

That last parenthetical reminded me that I'm really talking
about 'repertoire' here, which ISO SQL treats as a separate topic from
'encoding'. Exactly how an xml or jsonb type is represented internally
might be none of my business (unless I am developing a binary-capable
driver), but it's fair to ask what its repertoire is, and whether that's
full Unicode or not, and if not, whether the repertoire changes when some
server setting does.

I also think in that ideal world, or even this one, you could want
some way to query the catalogs to answer that basic question
about some given type.

Am I right that we simply don't have that? I currently answer such questions
by querying the catalog for the type's _send or _recv function name, then
going off to read the code, but that's hard to automate.

Regards,
-Chap

  3. Re: Dubious usage of TYPCATEGORY_STRING

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2021-12-07T15:48:21Z

    On 02.12.21 22:22, Tom Lane wrote:
> My first thought about fixing point 1 was to put "char" into some
> other typcategory, but that turns out to break some of psql's
> catalog queries, with results like:
> 
> regression=# \dp
> ERROR:  operator is not unique: unknown || "char"
> LINE 16:            E' (' || polcmd || E'):'
>                            ^
> HINT:  Could not choose a best candidate operator. You might need to add explicit type casts.

Could we add explicit casts (like polcmd::text) here?  Or would it break 
too much?

  4. Re: Dubious usage of TYPCATEGORY_STRING

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-07T15:51:46Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 02.12.21 22:22, Tom Lane wrote:
>> My first thought about fixing point 1 was to put "char" into some
>> other typcategory, but that turns out to break some of psql's
>> catalog queries, with results like:
>> 
>> regression=# \dp
>> ERROR:  operator is not unique: unknown || "char"
>> LINE 16:            E' (' || polcmd || E'):'
>> ^
>> HINT:  Could not choose a best candidate operator. You might need to add explicit type casts.

> Could we add explicit casts (like polcmd::text) here?  Or would it break 
> too much?

I assumed it'd break too much to consider doing that.  But I suppose
that since a typcategory change would be initdb-forcing anyway, maybe
it's not out of the question.  I'll investigate and see exactly how
many places would need an explicit cast.

			regards, tom lane

  5. Re: types reliant on encodings [was Re: Dubious usage of TYPCATEGORY_STRING]

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2021-12-07T15:52:42Z

    On 03.12.21 19:42, Chapman Flack wrote:
> Is there any way to find out, from the catalogs or in any automatable way,
> which types are implemented with a dependence on the database encoding
> (or on some encoding)?

What is this needed for?  C code can internally do whatever it wants, 
and the database encoding is effectively a constant, so there is no need 
for server-side code to be very much concerned about whether types do this.

Also, "types" is perhaps the wrong subject here.  Types only contain 
input and output functions and a few more bits.  Additional functions 
operating on the type could look at the server encoding without the type 
and its core functions knowing about it.

  6. Re: Dubious usage of TYPCATEGORY_STRING

    Robert Haas <robertmhaas@gmail.com> — 2021-12-07T15:59:45Z

    On Thu, Dec 2, 2021 at 4:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> An example of the reasons not to treat these types as being
> general-purpose strings can be seen at [1], where the "char"
> type has acquired some never-intended cast behaviors.  Taking
> that to an extreme, we currently accept
>
> regression=# select '(1,2)'::point::"char";
>  char
> ------
>  (
> (1 row)

What's wrong with that?

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  7. Re: Dubious usage of TYPCATEGORY_STRING

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-07T17:19:41Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Dec 2, 2021 at 4:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> An example of the reasons not to treat these types as being
>> general-purpose strings can be seen at [1], where the "char"
>> type has acquired some never-intended cast behaviors.  Taking
>> that to an extreme, we currently accept
>> 
>> regression=# select '(1,2)'::point::"char";
>> char
>> ------
>> (
>> (1 row)

> What's wrong with that?

Well, we don't allow things like

regression=# select '(1,2)'::point::float8;
ERROR:  cannot cast type point to double precision
LINE 1: select '(1,2)'::point::float8;
                             ^

It's not very clear to me why "char" should get a pass on that.
We allow such cases when the target is text/varchar/etc, but
the assumption is that the textual representation is sufficient
for your purposes.  It's hard to claim that just the first
byte is a useful textual representation.

Worse, PG is actually treating this as an assignment-level cast,
so we accept this:

regression=# create table t(f1 "char");
CREATE TABLE
regression=# insert into t values ('(1,2)'::point);
INSERT 0 1
regression=# table t;
 f1 
----
 (
(1 row)

I definitely don't think that should have worked without
any complaint.

			regards, tom lane

  8. Re: Dubious usage of TYPCATEGORY_STRING

    Robert Haas <robertmhaas@gmail.com> — 2021-12-07T18:04:50Z

    On Tue, Dec 7, 2021 at 12:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > What's wrong with that?
>
> Well, we don't allow things like
>
> regression=# select '(1,2)'::point::float8;
> ERROR:  cannot cast type point to double precision
> LINE 1: select '(1,2)'::point::float8;
>                              ^
>
> It's not very clear to me why "char" should get a pass on that.
> We allow such cases when the target is text/varchar/etc, but
> the assumption is that the textual representation is sufficient
> for your purposes.  It's hard to claim that just the first
> byte is a useful textual representation.

Fair enough, I guess. I am pretty skeptical of the merits of refusing
an explicit cast. If I ran the zoo, I would probably choose to allow
all such casts and make them coerce via IO when no other pathway is
available. But I get that's not our policy.

> Worse, PG is actually treating this as an assignment-level cast,
> so we accept this:
>
> regression=# create table t(f1 "char");
> CREATE TABLE
> regression=# insert into t values ('(1,2)'::point);
> INSERT 0 1
> regression=# table t;
>  f1
> ----
>  (
> (1 row)
>
> I definitely don't think that should have worked without
> any complaint.

Yes, that one's a bridge too far, even for me.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  9. Re: Dubious usage of TYPCATEGORY_STRING

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-07T20:24:46Z

    I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> Could we add explicit casts (like polcmd::text) here?  Or would it break 
>> too much?

> I assumed it'd break too much to consider doing that.  But I suppose
> that since a typcategory change would be initdb-forcing anyway, maybe
> it's not out of the question.  I'll investigate and see exactly how
> many places would need an explicit cast.

Um, I definitely gave up too easily there.  The one usage in \dp
seems to be the *only* thing that breaks in describe.c, and pg_dump
doesn't need any changes so far as check-world reveals.  So let's
just move "char" to another category, as attached.

			regards, tom lane

  10. Re: Dubious usage of TYPCATEGORY_STRING

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2021-12-09T15:27:31Z

    On 07.12.21 21:24, Tom Lane wrote:
> I wrote:
>> Peter Eisentraut<peter.eisentraut@enterprisedb.com>  writes:
>>> Could we add explicit casts (like polcmd::text) here?  Or would it break
>>> too much?
>> I assumed it'd break too much to consider doing that.  But I suppose
>> that since a typcategory change would be initdb-forcing anyway, maybe
>> it's not out of the question.  I'll investigate and see exactly how
>> many places would need an explicit cast.
> Um, I definitely gave up too easily there.  The one usage in \dp
> seems to be the*only*  thing that breaks in describe.c, and pg_dump
> doesn't need any changes so far as check-world reveals.  So let's
> just move "char" to another category, as attached.

Looks good to me.

  11. Re: Dubious usage of TYPCATEGORY_STRING

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-11T19:12:06Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 07.12.21 21:24, Tom Lane wrote:
>> Um, I definitely gave up too easily there.  The one usage in \dp
>> seems to be the*only*  thing that breaks in describe.c, and pg_dump
>> doesn't need any changes so far as check-world reveals.  So let's
>> just move "char" to another category, as attached.

> Looks good to me.

Pushed, thanks for reviewing.

			regards, tom lane

  12. TYPCATEGORY_{NETWORK,USER} [was Dubious usage of TYPCATEGORY_STRING]

    Chapman Flack <chap@anastigmatix.net> — 2022-01-03T18:36:52Z

    On 12/02/21 16:22, Tom Lane wrote:
> taking the special types out of the string category, so 0002
> attached invents a separate TYPCATEGORY_INTERNAL category to
> put them in.

On the same general topic, was there a deliberate choice to put
inet and cidr in TYPCATEGORY_NETWORK but macaddr and macaddr8
in TYPCATEGORY_USER?

It looks like macaddr was put in category U (macaddr8 didn't exist yet)
in bac3e83, the same commit that put inet and cidr into category I,
apparently in order to "hew exactly to the behavior of the previous
hardwired logic", on the principle that "any adjustment of the standard
set of categories should be done separately".

The birth of macaddr looks to have been back in 1998 in 2d69fd9, the
same commit that added 'ipaddr'. Neither was added at that time to
the hardcoded switch in TypeCategory(). The plot thickens....

ipaddr became inet in 8849655 (8 Oct 1998). cidr was added in 858a3b5
(21 Oct 1998).

Then ca2995 added NETWORK_TYPE to TypeCategory and put inet and cidr
in it (22 Oct 1998). Looks like that was done to reduce duplication
of pg_proc entries between inet and cidr by allowing implicit coercion.

And I guess you wouldn't want to suggest the existence of coercions
between MAC addresses and inet addresses.

But there aren't any such casts present in pg_cast anyway, so is that
a persuasive present-day rationale for the (otherwise odd-seeming) split
of these types across categories? They are grouped in a single
documentation "category".

Regards,
-Chap

  13. Re: TYPCATEGORY_{NETWORK,USER} [was Dubious usage of TYPCATEGORY_STRING]

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-01-03T18:55:35Z

    Chapman Flack <chap@anastigmatix.net> writes:
> On the same general topic, was there a deliberate choice to put
> inet and cidr in TYPCATEGORY_NETWORK but macaddr and macaddr8
> in TYPCATEGORY_USER?

Hard to say how "deliberate" it was, at this remove of time.

I do see an argument against reclassifying macaddr[8] into
TYPCATEGORY_NETWORK now: we generally expect that if a
category has a preferred type, any member type of the category
can be cast to that preferred type.  (The fact that OID is
marked preferred breaks that rule, but it holds pretty well
otherwise.)  I think this is why type interval has its own
category rather than being within TYPCATEGORY_DATETIME.

			regards, tom lane

  14. Re: TYPCATEGORY_{NETWORK,USER} [was Dubious usage of TYPCATEGORY_STRING]

    Chapman Flack <chap@anastigmatix.net> — 2022-01-03T19:54:18Z

    On 01/03/22 13:55, Tom Lane wrote:
> I do see an argument against reclassifying macaddr[8] into
> TYPCATEGORY_NETWORK now: we generally expect that if a
> category has a preferred type, any member type of the category
> can be cast to that preferred type.

I was wondering about the details of how that information gets used.
It seems partly redundant with what you learn from pg_cast. The
CREATE TYPE documentation says:

  The category and preferred parameters can be used to help control
  which implicit cast will be applied in ambiguous situations. ...
  For types that have no implicit casts to or from any other types,
  it is sufficient to leave these settings at the defaults. However,
  for a group of related types that have implicit casts, it is often
  helpful ...

which would suggest (to me on a first reading, anyway) that one starts
in pg_cast to find out what implicit casts, if any, exist, and then
looks to category and preferred if needed to resolve any ambiguity
that remains.

If understood that way, it doesn't seem to imply any ill effect of
having types within a category that might be partitioned into a few
disjoint subsets by "implicit cast exists between". (Such subsets
might be regarded as autodiscovered mini-categories.) But I could be
off-base to understand it that way.

Are there spots in the code where the expectation "if a category has
a preferred type, any member type of the category can be cast to that
preferred type" really takes that stronger form?

Hmm, I guess I can see some spots in Chapter 10, in the rules for
finding best-match operators or functions, or resolving UNION/CASE
types.

The UNION/CASE rules look like the effect might be benign: you have
step 4, inputs not of the same category => fail, then step 5, where
discovery of a preferred type can foreclose consideration of other
inputs, then step 6, implicit cast doesn't exist => fail. At first
blush, maybe that only fails the same cases that (if you treated
implicit-cast-related subsets within a category as mini-categories)
you would have failed in step 4.

The operator and function resolution rules seem harder to reason about,
and yeah, I haven't convinced myself their "any candidate accepts a
preferred type => discard candidates accepting non-preferred types" rules
couldn't end up discarding the part of the solution space where the
solution is, if disjoint "mini-categories" exist.

Regards,
-Chap