Re: [BUGS] Failure to coerce unknown type to specific type

Hello, I think this is a bug.

The core of this problem is that coerce_type() fails for Var of
type UNKNOWNOID.

The comment for the function says that,

> * The caller should already have determined that the coercion is possible;
> * see can_coerce_type.

But can_coerce_type() should say it's possible to convert from
unknown to any type as it doesn't see the target node type. I
think this as an inconsistency between can_coerce_type and
coerce_type. So making this consistent would be right way.

Concerning only this issue, putting on-the-fly conversion for
unkown nonconstant as attached patch worked for me. I'm not so
confident on this, though..

regards,

At Wed, 22 Apr 2015 23:26:43 -0700, Jeff Davis <pgsql@j-davis.com> wrote in <1429770403.4604.22.camel@jeff-desktop>
> On Wed, 2015-04-22 at 20:35 -0700, David G. Johnston wrote:
> 
> > But the fact that column "b" has the data type "unknown" is only a
> > warning - not an error.
> > 
> I get an error:
> 
> postgres=# SELECT '  '::text = 'a';
>  ?column? 
> ----------
>  f
> (1 row)
> 
> postgres=# SELECT a=b FROM (SELECT ''::text, '  ') x(a,b);
> ERROR:  failed to find conversion function from unknown to text
> 
> So that means the column reference "b" is treated differently than the
> literal. Here I don't mean a reference to an actual column of a real
> table, just an identifier ("b") that parses as a columnref.
> 
> Creating the table gives you a warning (not an error), but I think that
> was a poor example for me to choose, and not important to my point.
> > 
> > This seems to be a case of the common problem (or, at least recently
> > mentioned) where type conversion only deals with data and not context.
> > 
> > 
> > http://www.postgresql.org/message-id/CADx9qBmVPQvSH3
> > +2cH4cwwPmphW1mE18e=WUmLFUC-QZ-t7Q6Q@mail.gmail.com
> > 
> > 
> I think that is a different problem. That's a runtime type conversion
> error (execution time), and I'm talking about something happening at
> parse analysis time.
> 
> > 
> > but this too works - which is why the implicit cast concept above
> > fails (I'm leaving it since the thought process may help in
> > understanding):
> > 
> > 
> > SELECT 1 = '1';
> > 
> > 
> > From which I infer that an unknown literal is allowed to be fed
> > directly into a type's input function to facilitate a direct coercion.
> 
> Yes, I believe that's what's happening. When we use an unknown literal,
> it's acting more like a value constructor and will pass it to the type
> input function. When it's a columnref, even if unknown, it tries to cast
> it and fails.
> 
> But that is very confusing. In the example at the top of this email, it
> seems like the second query should be equivalent to the first, or even
> that postgres should be able to rewrite the second into the first. But
> the second query fails where the first succeeds.
> 
> 
> > At this point...backward compatibility?
> 
> Backwards compatibility of what queries? I guess the ones that return
> unknowns to the client or create tables with unknown columns?
> 
> > create table a(u) as select '1';
> > 
> > 
> > WARNING: "column "u" has type "unknown"​
> > DETAIL:  Proceeding with relation creation anyway.
> > 
> > 
> > Related question: was there ever a time when the above failed instead
> > of just supplying a warning?
> 
> Not that I recall.
> 
> 
> 
> > ​My gut reaction is if you feel strongly enough to add some additional
> > documentation or warnings/hints/details related to this topic they
> > probably would get put in; but disallowing "unknown" as first-class
> > type is likely to fail to pass a cost-benefit evaluation.
> 
> I'm not proposing that we eliminate unknown. I just think columnrefs and
> literals should behave consistently. If we really don't want unknown
> columnrefs, it seems like we could at least throw a better error.
> 
> If we were starting from scratch, I'd also not return unknown to the
> client, but we have to worry about the backwards compatibility.
> 
> > Distinguishing between "untyped" literals and "unknown type" literals
> > seems promising concept to aid in understanding the difference in the
> > face of not being able (or wanting) to actually change the behavior.
> 
> Not sure I understand that proposal, can you elaborate?

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index a4e494b..b64d40b 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -221,7 +221,7 @@ coerce_type(ParseState *pstate, Node *node,            return node;        }    }
-    if (inputTypeId == UNKNOWNOID && IsA(node, Const))
+    if (inputTypeId == UNKNOWNOID)    {        /*         * Input is a string constant with previously undetermined
type.Apply
 
@@ -275,6 +275,29 @@ coerce_type(ParseState *pstate, Node *node,        targetType = typeidType(baseTypeId);
+        /* Perform on the fly conversion for non-constants */
+        if(!IsA(node, Const))
+        {
+            Form_pg_type typform = (Form_pg_type) GETSTRUCT(targetType);
+            Node *result = 
+                (Node*) makeFuncExpr(typform->typinput,
+                         targetTypeId,
+                         list_make3(node,
+                                    makeConst(OIDOID, -1, InvalidOid,
+                                              sizeof(Oid),
+                                              ObjectIdGetDatum(InvalidOid),
+                                              false, true),
+                                    makeConst(INT4OID, -1, InvalidOid,
+                                              sizeof(uint32),
+                                              Int32GetDatum(inputTypeMod),
+                                              false, true)),
+                          InvalidOid, InvalidOid,
+                          COERCE_IMPLICIT_CAST);
+            ReleaseSysCache(targetType);
+
+            return result;
+        }
+        newcon->consttype = baseTypeId;        newcon->consttypmod = inputTypeMod;        newcon->constcollid =
typeTypeCollation(targetType);