Hi jian,
> Another issue is
>
> src1=# select cast(NULL::text as unknown);
> ERROR: cannot cast type unknown to text
> ...
> The error cursor points to the word "cast", the statement is "cast
> text as unknown"
> and the error message is "cannot cast type unknown to text".
> So the whole thing is very confusing for regular users.
> Maybe removing parser_coercion_errposition?
I tried that on master (89829354de1), with the same query and ten
others that reach this error, before and after. The position turns
out to be the useful part. It points at the expression whose type is
unknown, which is what the user has to change:
select a, c, cast(b as unknown) as bad, c + 1 from t;
ERROR: cannot cast type unknown to text
LINE 1: select a, c, cast(b as unknown) as bad, c + 1 from t;
^
create view v as
select a,
c,
cast(b as unknown) as bad
from t;
ERROR: cannot cast type unknown to text
LINE 4: cast(b as unknown) as bad
^
Without parser_coercion_errposition() every case prints only the
ERROR line, so in a view like that there is nothing to say which
column it is.
What is confusing in your example is the "to text": that conversion
is the one that resolves the output column, and the user never wrote
it. A hint that says what to do covers that, and keeps the position:
ERROR: cannot cast type unknown to text
LINE 1: select cast(NULL::text as unknown);
^
HINT: Cast the expression to the type you want directly, not to
type unknown.
transformAssignedExpr() does the same for a type mismatch: it keeps
the position and adds "You will need to rewrite or cast the
expression."
I also tried an errdetail saying that only a literal of type unknown
can be converted to another type, and dropped it because it isn't
true: parameters of type unknown are converted too.
PREPARE p(unknown) AS SELECT $1::text works, before and after the
patch.
With the hint, make check passes (239), with no new warnings. One
case has no position with any wording, a UNION branch:
select a from t union all select cast(b as unknown) from t;
The set-operation coercion does not pass a location. That is how it
is today and I left it alone.
> I guess I understand your comments, below are the comments I came up
> with.
> What do you think?
It reads well, and what it says about can_coerce_type() is right: it
does "if (inputTypeId == UNKNOWNOID) continue;". One thing I would
add, for the same reason as the errdetail above: parameters of type
unknown are also handled before this point, through
p_coerce_param_hook, so "Unknown-type literals and parameters were
coerced above."
The SQL and the output of each variant are attached.
Regards,
Manu
