raster and vector overloading each others signatures

[pramsey]

Our run_test harness does not test our geometry functions in the presence of the raster functions, and so we now have an 'unknown' signature collision problem similar to the one we had for geography:

up=# SELECT 'contains100', ST_contains('POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))', 'POINT(5 5)');
ERROR:  function st_contains(unknown, unknown) is not unique

In geography we decided to default to letting geometry always win the unknown collisions, so we added extra functions in all the cases where the functions had the same signatures:

CREATE OR REPLACE FUNCTION ST_Intersects(text, text)
	RETURNS boolean AS
	$$ SELECT ST_Intersects($1::geometry, $2::geometry);  $$
	LANGUAGE 'sql' IMMUTABLE ;

It seems like we need to find the collisions in raster and add over-rides there too. This issue also causes failures when you use run_test in 'extensions' mode, since in *that* case, the raster and geometry modules *are* tested simultaneously.

[robe]

hey I just committed a similar ticket at #2686. Though I consider this something we should not fix. the reason being is that I always thought counting on autocasting very bad to encourage so the fix to me should be done in our tests. We did it in 1.5 because we weren't allowed to introduce things that could break existing code. In 2.0 I accepted that there would be cases of sloppy coding that people would need to fix. So in my mind we SHOULD NOT CREATE OVERLOAD FUNCTIONS.

In fact we have more serious issues with geography/geometry that people accidentally use ST_DWithin, ST_Intersects in combo with them — and its a more pernicious problem because you get no error — you just might get the wrong answer and your query doesn't use an index.

[pramsey]

Patch fixes everything on the geometry side except the legacy.sql error, which is caused by the conflict between the system installed DLL and the regression one. Could back-port the GUC fix from trunk to get rid of that final error.

[robe]

pramsey — you have to take those out . Those are introducing new functions in a micro release — NOT ALLOWED. you can put them in 2.2 (though I would advice against it) but not in 2.1.

You may think those are harmless adds, but I don't think they are.

What if someone did this:

SELECT st_overlaps(someraster, '000000d23232')

What would you want to happen — it error or raster gets converted to geometry. I personally would prefer the erroring.

[robe]

ah never mind that example I guess that wasn't a good one. I still think you should take those out though.

[strk]

We did intentionally drop implicit text::geometry casts in the past indeed, or so I recall. Not sure which way I'd prefer, but I can feel Regina being pretty confident on her argument …

One thing I can tell: for geography we were kind of _pushed_ to provide overrides because geography was _forced_ to the user on upgrade. With EXTENSION, _raster_ is also forced, which puts it in the same situation as the geography one.

But it's so long that this problem exists (2.0.0 ?) that I would not consider it a regression. So, I'd rather tweak the tests to explicitly cast than introduce overloads right now. Especially for the 2.1 branch.

[robe]

test tweak is in #2686

[pramsey]

Patched tests in 2.1 at r12373, in trunk at r12374