Re: Reported type mismatch improperly

On Sat, Jul 18, 2020 at 12:23 AM David G. Johnston <
david(dot)g(dot)johnston(at)gmail(dot)com> wrote:

> 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
>

I think you are saying about "select null::init", I forget this before..
I agree
with the "benefit for effort" judgment, This would be good if it can remove
some "tech debt" and without introducing new problems.

> - which they still need to be aware of even though this patch removes one
> situation where it matters.
>

Can you provide an example of this? If it is not very troublesome, I'd
like to fix both.

>
>> 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.
>
>

The above patch is just to see if there are any objections. I'd try to fix
the
above issues on the official patch. Thank you for your feedback!

--
Best Regards
Andy Fan