Re: BUG #5974: UNION construct type cast gives poor error message

The following bug has been logged online:

Bug reference: 5974
Logged by: Jeff Wu
Email address: jwu(at)atlassian(dot)com
PostgreSQL version: 9.0
Operating system: Mac OS X
Description: UNION construct type cast gives poor error message
Details:

The UNION construct (as noted on this page:
http://www.postgresql.org/docs/9.0/static/typeconv-union-case.html) will
cast unknown types to TEXT, however, if you try to do three or more UNIONs
the order in which the UNIONs are executed will cause some columns to be
cast to TEXT prematurely. The result is a type mismatch error.

For example:
SELECT 1,null,null
UNION
SELECT 2,3,null
UNION
SELECT 3,null,4

will fail while

SELECT 1,null,null::INTEGER
UNION
SELECT 2,3,null
UNION
SELECT 3,null,4

will succeed.

This is not a critical error, but I would say that the error message is
misleading because it is not obvious that Postgres casts unknown columns to
TEXT automatically.

The current error message is:
ERROR: UNION types text and integer cannot be matched

I would suggest something like:
ERROR: UNION types text and integer cannot be matched. HINT: Postgres casts
unknown types to TEXT by default.

Thanks,

Jeff

"Jeff Wu" <jwu(at)atlassian(dot)com> writes:
> The UNION construct (as noted on this page:
> http://www.postgresql.org/docs/9.0/static/typeconv-union-case.html) will
> cast unknown types to TEXT, however, if you try to do three or more UNIONs
> the order in which the UNIONs are executed will cause some columns to be
> cast to TEXT prematurely. The result is a type mismatch error.

Really the *right* fix for this would be to resolve the common type
just once across the whole nest of set operations. That wouldn't be
terribly difficult from a coding standpoint, I think. The reason we
haven't done it is that it looks like the SQL standard requires type
resolution for set-ops to happen one pair of input relations at a time.
See SQL:2008 7.13 <query expression>, in which everything that's said
about UNION/INTERSECT/EXCEPT is phrased in terms of exactly two input
subqueries; for instance INTERSECT's result type is defined in syntax
rule 18b as:

The declared type of the i-th column of TR is determined by
applying Subclause 9.3, "Result of data type combinations", to
the declared types of the i-th column of T1 and the i-th column
of T2.

If anyone can think of a way to read the spec to allow subclause 9.3 to
be applied to the whole set of columns at once, we could make this work
less surprisingly. Or maybe we could find out that some other products
do it like that despite what the spec says?

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> The reason we haven't done it is that it looks like the SQL
> standard requires type resolution for set-ops to happen one pair
> of input relations at a time.

Well, it also requires that an unadorned quoted literal is of type
char(n). This is inextricably tied in with the PostgreSQL deviation
from standard handling of literals so that user-defined types can be
more gracefully handled.

>From my perspective the "right" answer is to be able to resolve two
unknown types to unknown rather than text in a few places where we
are currently compelled to assign a concrete type. This just seems
odd and wrong:

test=# select pg_typeof((select '1' limit '1'));
pg_typeof
-----------
unknown
(1 row)

test=# select pg_typeof((select '2' limit '1'));
pg_typeof
-----------
unknown
(1 row)

test=# select pg_typeof((select '1' union all select '2' limit
'1'));
pg_typeof
-----------
text
(1 row)

Likewise with the CASE predicate and it abbreviated forms (e.g.,
COALESCE).

I remember looking at this a couple years ago and finding that
making that work was more time than I could throw at it just then,
but I'm convinced that several warts on the type handling which
generate posts on this list now and again would best be dealt with
that way.

At least part of the previous discussion was on the thread which
starts here:

http://archives.postgresql.org/pgsql-hackers/2009-09/msg00388.php

As I recall, one issue is that as the code is currently organized,
some of this type resolution would need to be deferred to execution
time. :-(

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> From my perspective the "right" answer is to be able to resolve two
> unknown types to unknown rather than text in a few places where we
> are currently compelled to assign a concrete type.

Well, it's not so easy as that. Consider

select '1' union select '1 ';

How many rows does that produce? You cannot answer without imputing a
data type to the columns. "text" will give a different answer than
"integer" or "bpchar".

It's possible that we could make UNION ALL act differently from all
other set-operations, and refrain from resolving a type for the single
case of UNION ALL; but I can't say that I care for that idea, or see any
support for it in the standard. AFAICS the standard says that output
data types are to be resolved in the same way for all set operations.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Consider
>
> select '1' union select '1 ';
>
> How many rows does that produce? You cannot answer without
> imputing a data type to the columns. "text" will give a different
> answer than "integer" or "bpchar".

Well, if we were to assign both to type unknown initially, we would
clearly need to resolve that before execution, But I'm not
expecting that such execution would happen before analyzing the rest
of the query. If the above is on the left side of a union with

select 1;

the unknown could then be resolved to integer. I expect that all of
this should happen before any of the unions is *executed*. Perhaps
I'm arguing for the same thing you were, but just have my head
tilted at a different angle?

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Consider
>>
>> select '1' union select '1 ';
>>
>> How many rows does that produce? You cannot answer without
>> imputing a data type to the columns. "text" will give a different
>> answer than "integer" or "bpchar".

> Well, if we were to assign both to type unknown initially, we would
> clearly need to resolve that before execution, But I'm not
> expecting that such execution would happen before analyzing the rest
> of the query. If the above is on the left side of a union with

> select 1;

> the unknown could then be resolved to integer. I expect that all of
> this should happen before any of the unions is *executed*. Perhaps
> I'm arguing for the same thing you were, but just have my head
> tilted at a different angle?

Yes, I think you are saying the exact same thing I am, just phrased
differently: you wish that in

(select '1' union select '2') union select 3

the fact that the third value is clearly integer would influence
the choice of the resolved type of the first UNION. My vision of
how to implement that is different than what you seem to have in
mind, but it would come out with the same answer. The sticking point
is just that in purely syntactic terms this is action-at-a-distance,
and so it's hard to square with the spec. I think that our current
reading (in which the '1' and '2' get resolved as text) is actually
closer to what the spec says.

For those following along at home, there is another issue involved
here, which is our choice to treat string-literal constants the same
as NULL constants --- they're both UNKNOWN so far as the type resolution
rules go. It's not that surprising, perhaps, that (select '1' union
select '2') is resolved as text, but newbies tend to not think that
NULL ought to act like that. However, so far as I can see the spec
simply disallows a not-explicitly-cast NULL constant in cases like
this, which seems if anything even less friendly than what we're doing.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> you wish that in
>
> (select '1' union select '2') union select 3
>
> the fact that the third value is clearly integer would influence
> the choice of the resolved type of the first UNION.

Yeah.

> My vision of how to implement that is different than what you seem
> to have in mind, but it would come out with the same answer.

OK

> The sticking point is just that in purely syntactic terms this is
> action-at-a-distance, and so it's hard to square with the spec. I
> think that our current reading (in which the '1' and '2' get
> resolved as text) is actually closer to what the spec says.

Would the approach you have in mind accept a query which is valid
under the spec yet return different results? If not, we can
legitimately call it an extension.

If we're not going to do that, there's probably some room to improve
our error reporting and/or documentation around this issue.

-Kevin

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> so far as I can see the spec simply disallows a
> not-explicitly-cast NULL constant in cases like this, which seems
> if anything even less friendly than what we're doing.

Just to illustrate the current behavior:

test=# select null union select 1;
?column?
----------
1

(2 rows)

test=# select null union select null union select 1;
ERROR: UNION types text and integer cannot be matched
LINE 1: select null union select null union select 1;
^
test=# select null union (select null union select 1);
?column?
----------

1
(2 rows)

So, we're talking about making the second of these three cases work,
too. I'm not sure the spec requires *any* of them to work.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> The sticking point is just that in purely syntactic terms this is
>> action-at-a-distance, and so it's hard to square with the spec. I
>> think that our current reading (in which the '1' and '2' get
>> resolved as text) is actually closer to what the spec says.

> Would the approach you have in mind accept a query which is valid
> under the spec yet return different results? If not, we can
> legitimately call it an extension.

Well, the case that is bothering me is stuff like

(select '1' union select '1 ') union all select 2;

The first union produces 1 row if you resolve the constants as integers,
but 2 rows if you resolve as text, which I think is what the spec would
expect here. And since the second union has ALL, that makes a
difference to the final output. Now in this particular case we'd fail
with "UNION types text and integer cannot be matched" so you never get
as far as noticing what the runtime behavior is.

[ experiments a bit... ] You can show a difference in results with
this:

regression=# (select '1' union select '1 ') union all select '2'::bpchar;
?column?
----------
1
1
2
(3 rows)

This produces 3 rows because the UNION resolves as text, but what we're
discussing here would allow it to resolve as bpchar, which would have
different behavior:

regression=# (select '1' union select '1 '::bpchar) union all select '2'::bpchar;
?column?
----------
1
2
(2 rows)

It's debatable about how important this is, and one could also say that
the behavior of our bpchar is not entirely standards compliant in the
first place, so maybe this isn't a compelling example. But I'm worried
that there may be related cases where it's a bigger deal.

regards, tom lane

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> I'm not sure the spec requires *any* of them to work.

It doesn't. NULL in the standard is not part of the generic expression
syntax; it only appears as <implicitly typed value specification> (which
lets it be the direct argument of CAST, or the DEFAULT value for a table
column) and as <contextually typed value specification> (which lets it
be an INSERT or UPDATE's direct source value). So far as I can see,
these are the only contexts where a NULL literal is allowed in SQL99.
The syntax rule for a NULL literal is interesting too:

2) The declared type DT of a <null specification> NS is determined
by the context in which NS appears. NS is effectively replaced
by CAST ( NS AS DT ).

NOTE 70 - In every such context, NS is uniquely associated with
some expression or site of declared type DT, which thereby
becomes the declared type of NS.

The NOTE makes it pretty clear that this is intentional and not some
sort of omission. I have not tried to trace the syntax productions to
verify that this is still true in SQL:2008 (mainly because I don't have
an easily greppable copy of that standard) but I see that the above text
is still there in 6.5 <contextually typed value specification>.

In short: the standard disallows an un-cast NULL in a SELECT's targetlist
a priori, let alone one in a SELECT that's an arm of a set operation.
I doubt we want to copy that ...

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Well, the case that is bothering me is stuff like
>
> (select '1' union select '1 ') union all select 2;
>
> The first union produces 1 row if you resolve the constants as
> integers, but 2 rows if you resolve as text, which I think is what
> the spec would expect here.

The way I have read it, the spec would make those first two literals
char(1) and char(2), and the trailing space would be ignored in an
equality comparison between those. But you could make your point
with a leading space, I think.

> Now in this particular case we'd fail with "UNION types text and
> integer cannot be matched" so you never get as far as noticing
> what the runtime behavior is.

Right, which makes it OK to provide something which *does* work here
as an extension.

> [ experiments a bit... ] You can show a difference in results
> with this:
>
> regression=# (select '1' union select '1 ') union all select
> '2'::bpchar;
> ?column?
> ----------
> 1
> 1
> 2
> (3 rows)
>
> This produces 3 rows because the UNION resolves as text, but what
> we're discussing here would allow it to resolve as bpchar, which
> would have different behavior:
>
> regression=# (select '1' union select '1 '::bpchar) union all
> select '2'::bpchar;
> ?column?
> ----------
> 1
> 2
> (2 rows)

Which would be the right answer according to the spec, although that
seems to be sort of an accident here.

> It's debatable about how important this is, and one could also say
> that the behavior of our bpchar is not entirely standards
> compliant in the first place, so maybe this isn't a compelling
> example. But I'm worried that there may be related cases where
> it's a bigger deal.

We are in territory where the choice to treat literals as type
unknown where the spec requires bpchar will probably lead to *some*
corner cases where behavior is nonstandard no matter what we do. I
think the best we can do here is (in what I think is order of
importance):

(1) Try not to break anything which works for current PostgreSQL
queries.

(2) Try not to add any additional behavioral differences from the
standard where a query now runs without error with
standard-conforming results.

(3) Try to maintain some coherent handling for unknown values. I
think that's currently lacking when the first of these fails and the
others work:

select null union select null union select 1;
select null union (select null union select 1);
select null union select 1 union select null;

Likewise, the first of these fails and the others don't:

select '1' union select '1 ' union select 1;
select '1' union (select '1 ' union select 1);
select '1' union select 1 union select '1 ';

Explaining that could be tough. I'm arguing that the first line
should be made to work like the others in terms of type resolution.
Since that case now throws and error under both the standard and
current PostgreSQL releases, it's OK as an extension.

-Kevin

On 13/04/11 19:32, Tom Lane wrote:
> "Jeff Wu"<jwu(at)atlassian(dot)com> writes:
>> The UNION construct (as noted on this page:
>> http://www.postgresql.org/docs/9.0/static/typeconv-union-case.html) will
>> cast unknown types to TEXT, however, if you try to do three or more UNIONs
>> the order in which the UNIONs are executed will cause some columns to be
>> cast to TEXT prematurely. The result is a type mismatch error.
> <snip>
>
> Or maybe we could find out that some other products
> do it like that despite what the spec says?
>
> regards, tom lane

I happen to have a MS SQLServer 2008 instance at work as well as a MySQL
5.1 and an Oracle 10g. With the query:

SELECT 1,null,null
UNION
SELECT 2,3,null
UNION
SELECT 3,null,4

In MS SQLServer I get (NB: no column headings):

----------------------
----------------------
1 | <null> | <null>
2 | 3 | <null>
3 | <null> | 4

In MySQL I get:
--------------------
1 | NULL | NULL
--------------------
1 | <null> | <null>
2 | 3 | <null>
3 | <null> | 4

In Oracle I get a delicious error message:

Error: ORA-00923: FROM keyword not found where expected

SQLState: 42000
ErrorCode: 923
Position: 19

Regards,

--
Mike Fowler
Registered Linux user: 379787

Mike Fowler <mike(at)mlfowler(dot)com> wrote:

> SELECT 1,null,null
> UNION
> SELECT 2,3,null
> UNION
> SELECT 3,null,4

> In Oracle I get a delicious error message:
>
> Error: ORA-00923: FROM keyword not found where expected

For Oracle, shouldn't that be:

SELECT 1,null,null FROM DUAL
UNION
SELECT 2,3,null FROM DUAL
UNION
SELECT 3,null,4 FROM DUAL

-Kevin

On 14/04/11 17:05, Kevin Grittner wrote:
> SELECT 1,null,null FROM DUAL
> UNION
> SELECT 2,3,null FROM DUAL
> UNION
> SELECT 3,null,4 FROM DUAL
Sadly I can't profess to knowing Oracle, however if I run the query as
suggested I get:

--------------------
1 | NULL | NULL
--------------------
1 | <null> | <null>
2 | 3 | <null>
3 | <null> | 4

So to summarise, Oracle and PostgreSQL need minor tweaks to run cleanly
and SQLServer and MySQL do not. Given that the change for PostgreSQL is
so minor, I vote for changing the error message as Jeff suggests in the
interim to help users while the standards argument continues. Patch
attached.

Regards,

--
Mike Fowler
Registered Linux user: 379787

Mike Fowler <mike(at)mlfowler(dot)com> wrote:

> So to summarise, Oracle and PostgreSQL need minor tweaks to run
> cleanly and SQLServer and MySQL do not.

The FROM DUAL in Oracle has nothing to do with the issue at hand.
That is just because they always require a FROM clause on every
SELECT. DUAL is a special table with one row you can use when you
just want to select a literal.

That means that all three of the databases you tested have
extensions to the standard similar to what is being contemplated for
PostgreSQL. If nothing else, adding such an extension would ease
migration from those other products, but I think it would prevent
some user confusion and posts to the -bugs list, too.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>> So to summarise, Oracle and PostgreSQL need minor tweaks to run
>> cleanly and SQLServer and MySQL do not.

> That means that all three of the databases you tested have
> extensions to the standard similar to what is being contemplated for
> PostgreSQL.

Uh, no, it proves they all extend the standard to allow NULL to be
written without an immediate cast. Mike's test really fails to prove
anything about the point at hand, which is what data type is being
imputed to the inner UNION.

I don't know those other DBMSes well enough to suggest a test that would
be definitive on the point, though. We'd need something where the
choice of datatype is material to the final visible result, and at least
in PG that requires some knowledge of not-very-standard behaviors.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:

>> That means that all three of the databases you tested have
>> extensions to the standard similar to what is being contemplated
>> for PostgreSQL.
>
> Uh, no, it proves they all extend the standard to allow NULL to be
> written without an immediate cast. Mike's test really fails to
> prove anything about the point at hand, which is what data type is
> being imputed to the inner UNION.

The query run was:

SELECT 1,null,null
UNION
SELECT 2,3,null
UNION
SELECT 3,null,4

It's a bit of a stretch to think that the columns returned from the
final union weren't integer, or that integer is the default type of
the union of two nulls. It's anyone's guess at this point whether
the third column was unknown during the leftmost union and the type
set in the next union, or the set of columns involved in the union
were all evaluated as a group. If they don't have other literals of
unknown type it may be hard to discern the implementation details,
but either I've missed something or we're considering similar user
visible behavior.

> I don't know those other DBMSes well enough to suggest a test that
> would be definitive on the point, though. We'd need something
> where the choice of datatype is material to the final visible
> result, and at least in PG that requires some knowledge of
> not-very-standard behaviors.

If the implementation details for the other databases are that hard
to discern, how much do we care *how* they do it? It seems to me
that the important point here is that they don't throw an error on
that query and we do.

What am I missing?

-Kevin

So I'm a n00b to the open source community, but what needs to happen to get
this fix in?

On 14 April 2011 15:13, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:

> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
>
> >> That means that all three of the databases you tested have
> >> extensions to the standard similar to what is being contemplated
> >> for PostgreSQL.
> >
> > Uh, no, it proves they all extend the standard to allow NULL to be
> > written without an immediate cast. Mike's test really fails to
> > prove anything about the point at hand, which is what data type is
> > being imputed to the inner UNION.
>
> The query run was:
>
> SELECT 1,null,null
> UNION
> SELECT 2,3,null
> UNION
> SELECT 3,null,4
>
> It's a bit of a stretch to think that the columns returned from the
> final union weren't integer, or that integer is the default type of
> the union of two nulls. It's anyone's guess at this point whether
> the third column was unknown during the leftmost union and the type
> set in the next union, or the set of columns involved in the union
> were all evaluated as a group. If they don't have other literals of
> unknown type it may be hard to discern the implementation details,
> but either I've missed something or we're considering similar user
> visible behavior.
>
> > I don't know those other DBMSes well enough to suggest a test that
> > would be definitive on the point, though. We'd need something
> > where the choice of datatype is material to the final visible
> > result, and at least in PG that requires some knowledge of
> > not-very-standard behaviors.
>
> If the implementation details for the other databases are that hard
> to discern, how much do we care *how* they do it? It seems to me
> that the important point here is that they don't throw an error on
> that query and we do.
>
> What am I missing?
>
> -Kevin
>

--
Jeff Wu
Marketing Quant, Atlassian
(714) 319-7604

Jeff Wu <jwu(at)atlassian(dot)com> wrote:

> what needs to happen to get this fix in?

Well, "fix" implies that there is a bug, which there isn't. The
current behavior doesn't violate the requirements of the standard,
nor is it a regression from the behavior of any previous PostgreSQL
release. What we're talking about is a request for enhancement --
to return results for cases in which the standard allows an error to
be thrown.

Enhancements are not applied to stable branches, and the 9.1 release
has been in feature freeze for months now, so the earliest release
we'd probably consider is 9.2.

If there are no objections I'll add this to the TODO Wiki page with
a reference to Tom's description of how it should be implemented. I
didn't get the sense from the discussion that this is one which
should be marked "easy" -- agreed?

In the meantime, the workaround is to explicitly cast quoted
literals and nulls in the result list of a set operation like UNION.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Jeff Wu <jwu(at)atlassian(dot)com> wrote:
>> what needs to happen to get this fix in?

> Well, "fix" implies that there is a bug, which there isn't.

I think what Jeff was actually suggesting was that we commit the
proposed added HINT. I didn't like the hint patch at all --- as given,
the hint would be emitted in a lot of cases where it'd be irrelevant and
just plain distracting --- but it might be possible to develop an
improved version that would be committable in the 9.1 timeframe.

> If there are no objections I'll add this to the TODO Wiki page with
> a reference to Tom's description of how it should be implemented.

I think there might be something related there already, but if not,
feel free to add it.

regards, tom lane