Text autocasting behavior of geography is breaking geometry functionality

[robe]

I have always been against these autocasts especially the text ones. While I would like to remove it for geometry, that will of course break existing functionality, but PLEASE remove it for geography.

This breaks SELECT ST_AsBinary('POINT(1 2)');

and gives this error

ERROR: function st_asbinary(unknown) is not unique LINE 1: SELECT ST_AsBinary('POINT(1 2)')

HINT: Could not choose a best candidate function. You might need to add explicit type casts.

[pramsey]

How to disable it?

[robe]

Okay this is a more serious problem than I thought. We don't have an autocast for text. Its the fact that

'POINT(1..)' is unknown and not text.

SELECT ST_AsBinary('POINT(1 3)'::text)

works fine. I guess we'll have the same issue with WKTRaster. Not sure the solution to this without resorting to different naming :(

[robe]

It appears there is one other solution besides renaming functions and that is to declare yet another function that in the face of unknown trumps the other functions.

After studying the docs, it appears to me that the text/varcahr datatype is hard-wired to rank higher as an appropriate CAST for unknown than any other data type (or at least in 8.4).  http://www.postgresql.org/docs/8.4/interactive/typeconv-func.html (see bullet 4 item d) -- have to test 8.3 though docs of 8.3 suggest the same)

The below appears to solve the problem (for ST_AsBinary and I presume we could do the same for others where we will have name clashes). I've only tested on 8.4 (not yet on 8.3).

Alternatively we could do nothing and proclaim this is a bad habit of people anyway and force them to change their ways by breaking their malformed apps. It might make many people mad, but they might thank us in the long run (or maybe not :))

CREATE OR REPLACE FUNCTION st_asbinary(text)
  RETURNS bytea AS
$$ SELECT st_asbinary($1::geometry);  $$
  LANGUAGE 'sql' IMMUTABLE STRICT
  COST 1;

Does anyone see a problem with the aforementioned solution.

[kneufeld]

Your proposed function does seem to solve the problem, but we would need to add a textual overload for every function that also has a geography overload.

ie. this also currently breaks. SELECT ST_AsText('POINT(1 3)');

This means that geography would never benefit from the unknown->text autocast (Not that it's a big deal, cuz I too am against autocasts anyway). Additionally, whenever a new geography function was added, developers would need to remember to add the textual overide to point to the geometry method, avoiding ambiguity.

You are right. Not having these functions would likely be a breaking change for many applications.

... or we skip a 1.5 release and go straight to 2.0 where a breaking change is almost expected :)

[pramsey]

I've got a whole wagon-load of breaking changes for 2.0, so for 1.5 we'll put in the casts and perhaps at 2.0 we remove them.

[kneufeld]

works fer me.

[robe]

So what are we doing now? I was thinking 1.5 was just an interim to get a feel for this geography thing before we go full break in 2.0 and rip out all those deprecated functions etc. So I didn't want to cause havoc in 1.5 -- and still think we need the transition phase.

[robe]

hmm -- have we decided what course of action to take here. Paul and Kevin gave confusing signals and Mark you have given no signal what so ever :).

[kneufeld]

Confusing signals? I thought we were all on the same page. Paul mentioned that he has many breaking changes that he'll introduce in 2.0. So to handle this issue and avoid a breaking change in 1.5, we'll add the overloaded functions that accept textual arguments (which still complies with the new release policy). Then, when 2.0 rolls out, we can remove the unnecessary functions, enforcing explicit typecasting, following the lead of PostgreSQL.

I was merely pointing out that we would need to overload every function that shares a common prototype as geometry, not just st_asbinary.

So geography introduced at least one breaking change that looks like can be dealt with easy enough.

[robe]

That's what I thought you meant -- but got all confused when Paul started talking about adding Casts since these are function overloads and not casts.

So to summarize -- we'll add overloads for

ST_AsText, ST_AsBinary, ST_Intersects (when that is implemented), and ST_DWithin, ST_Distance also when that is implemented. Are there any others I'm missing that share same names?

I haven't checked out the operators && (I assume those are okay)

[robe]

Did this for ST_AsText and ST_AsBinary at r4540. Still need to do this for ST_AsKML, ST_AsGML, ST_AsSVG

[pramsey]

I've done this for all the signatures I think might realistically be exercised with a text input at r4762. Namely those sigs that have just a geometry for an argument, but not the multiple parameterized versions of AsGML, AsKML etc.