Re: Reported type mismatch improperly

On Fri, Jul 17, 2020 at 2:00 AM Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> wrote:

> Currently when we call select_common_type, it compares the 2 exprs, if the
> expr
> type of both are unknown, it will be set to TEXTOID with some
> reasons, which
> can cause the issue like below.
>
>
> postgres=# select null union all select null union all select 1;
> ERROR: UNION types text and integer cannot be matched
> LINE 1: select null union all select null union all select 1;
>
>
> In this case, we can't blame the user, they may want the nulls to be at
> the top
> of the result.
>

This seems like a win in terms of benefit for effort. Today we just
instruct users to cast the first null - which they still need to be aware
of even though this patch removes one situation where it matters.

> I worked on a patch to fix this, the main idea is before going to the above
> logic, I peak all the exprs for a given column first, and choose a default
> one
> when we see the Unknown & Unknown case(rather than TextOid),
>
> do you think it is ok?
>

The comment in parse_coerce.c needs to be updated.

I'm not sure what the general protocol here is for code comments and
pointing out that they are not good examples of English prose.
Documentation prose has a higher bar but I don't know how big the gap is.

make issues: variable previous_is_null set but not used warning

make installcheck passes

You should include tests for the other SETOPS if this is intended to work
for those as well. And maybe the failure mode where there are two non-null
possibilities in the tree and the first one matches while the second is of
a different type.

Nothing in the code itself stands out to me and I'm not going to mark this
ready for committer myself (though its not ready yet per the remarks above)
due to inexperience in C coding so another reviewer will be needed.

You should add this to the 2020-09 commitfest.

David J.