MSSQLSpatial incorrectly assumes an "image" type column to be geometry

[danhomerick]

When the geometry column is not explicitly named, the MS SQL Spatial format attempts to autodetect which column contains geometry data. The autodetection logic assumes that any "image" type column (a subtype of binary data) contains WKB. Because the "image" type, unlike the "geometry" and "geography" column types is not exclusive to spatial data, this assumption is risky.

At connection, the spatial storage format is specified (WKB, WKT, or Native), defaulting to Native. A patch is attached which propagates which storage format is being used down to the autodetection logic, and changes the logic so as to only make the risky assumption about an "image" column when the spatial data is stored in WKB. It also allows for WKB which is stored in a "varbinary" column type, since the "image" column type is deprecated (since SQL Server 2005 [1]).

A more comprehensive fix might be to attempt decoding some of the "image" column, in order to check that it actually contains WKB.

The patch does not address autodetecting a column containing WKT, which was also not addressed by the original code.

An alternate fix might be to remove autodetection of geometry columns entirely unless stored in the unambiguous "geometry" or "geography" types (i.e. native).

[1] ​http://msdn.microsoft.com/en-us/library/ms187993.aspx

[danhomerick]

The supplied patch is fundamentally flawed, and should be discarded. It was based on the incorrect assumption that the type name returned by SQLColAttribute (specifying the SQL_DESC_TYPE_NAME attribute) is the same as the type name used when the table was created (e.g. "image", "geometry" or "geography". It is not.

I was initially misled by the test table that I was working with, which had an "image" column, for which the type name did happen to match. I didn't test the patch sufficiently to realize my mistake until now. Mea culpa.

I'm not marking the issue as invalid, because the problem with a binary, non-spatial column being wrongly assumed to contain spatial data does still exist. But I wouldn't object if the issue was closed, and quietly buried. It's pretty embarrassing, really.

[tamas]

Could you provide the table definition which can lead to this condition?

[danhomerick]

Sure. Slightly shortened from what I was using, but:

CREATE TABLE [dbo].[Table1] (

[ID] [int] IDENTITY (1, 1) NOT NULL , [foo] [int] NULL , [bar] [image] NULL

) ON [PRIMARY] GO

Note that "bar" does not containing spatial data.

[tamas]

And how did you do the query against this non spatial table? I'm not sure the queries will require to work on such tables that don't have any geometry columns.

[danhomerick]

I don't have the exact code in front of me, but something like:

OGRLayer *layer = dataSource.executeSQL("SELECT * FROM Table1", NULL, NULL); OGRFeature *feature = layer->GetNextFeature(); int fooValue = feature->GetFieldAsInteger(0); int sizeOfBar; GByte *barValue = feature->GetFieldAsBinary(1, &sizeOfBar);

The documentation doesn't suggest that this won't work. More to the point, from a usability standpoint it would be really nice if we could get this to work. Since it's not using the "OGRSQL" dialect, it seems reasonable to hope that the SQL query could be "passed directly through to MSSQL" and get usable results.

[danhomerick]

Corrected for wiki formatting, and to include the id field:

OGRLayer *layer = dataSource.executeSQL("SELECT * FROM Table1", NULL, NULL);
OGRFeature *feature = layer->GetNextFeature();
int idValue = feature->GetFieldAsInteger(0);
int fooValue = feature->GetFieldAsInteger(1);
int sizeOfBar;
GByte *barValue = feature->GetFieldAsBinary(2, &sizeOfBar);

[tamas]

Fixed in trunk (r25989)