problems in SQL Server support

[FilipDeVos]

The implementation of the SQL Server dialect has some issues in my opinion.

ToSqlType

The datatypes are no always optimal in this function.

• Uint64 should be implemented as NUMERIC(20), current type overflows
• Float32 and Float64 are not 100% compatible with the real and float type from SQL Server (no infinity for example)
• NullBool can be implemented as bit, bit is nullable in sql server
• Time should be implemented as DATETIME2, the current type only has a 3 millisecond precision
• String should be stored as nvarchar (UCS-2 strings) instead of varchar (ansi strings).
• maxsize < 1 should use nvarchar(max) as type

Note: some of these types are only supported in SQL Server 2008 and higher, but I think we can start dropping support for 10 year old database versions.

TruncateClause

SQL Server supports the TRUNCATE TABLE clause when there are no foreign keys referencing the table, I suspect this is why it is implemented with DELETE FROM. I would implement a check if there are constraints, and if not use TRUNCATE TABLE.

QuoteField

The function QuoteField is currently implemented by wrapping names with a double quote. This only works when the database option SET QUOTED_IDENTIFIER ON is set. It also does not escape a double quote in the object name. The better approach would be to implement the functionality of the SQL Server QUOTENAME() function which wraps names with [ and ] and escapes a ] in the string by doubling it up.

QuotedTableForQuery

The schema and table name should be quoted

IfSchemaNotExists, IfTableExists and IfTableNotExists

It would be better to implement this with the schema_id() function. The current implementation does not work on a case sensitive SQL Server instance. The function will still be SQL injection vulnerable but I doubt this is an issue.

[pierreprinetti]

Thank you, I'm working on this.

[FilipDeVos]

I took a stab at it, but have not tested at all (I only have localdb on my pc atm which does not support tcp/ip connections)

See https://github.com/FilipDeVos/gorp/commit/0d3be59de0edc41351b99e251e91a799ebe1c1e6

[pierreprinetti]

Nice job. My attempt https://github.com/qrawl/gorp/blob/mssqlfix/dialect.go#L454 had some retrocompatibility added using a Version property on the dialect struct.

[FilipDeVos]

You can remove the check for maxsize. SQL Server 2005 also supports NVARCHAR(max).

if you want to limit, the maximum string lengh in a page on sql server is 4000 unicode characters.

[pierreprinetti]

You are right. Nonetheless, Acces2007 reads a 255<nvarchar as "memo" field, thus limiting query capabilities, hence the check. I would assume that those who use mssql2005 in 2015 are likely to use Acces as well.

[FilipDeVos]

Ok, I understand. That works for me.

[pierreprinetti]

It seems like you can't use a nvchar(max) field as primary key. Do you have such experience?

[FilipDeVos]

This is correct for many reasons. For starters, a primary key constraint in SQL Server is implemented as an index and an index in SQL Server has a maximum of 900 bytes as the sum of the field lengths of the index.

The second reason is that an index in SQL Server is implemented as a B-Tree of data pages that can contain 8K of data each. An nvarchar(max) field can contain up to 1.2GB of data, which is realized as a separate linked list of data pages. So your "field" in the table is actually a pointer to another linked list.

Now technically, it doesn't make much sense to index a long string blob. it makes a lot more sense to store a hash like sha1 generated from the string and index that.

For gorp I think it is acceptable to have to specify a max string length if you want to index the field.

[pierreprinetti]

Thank you, I learnt something new. I just wanted to point out that now a default value would be an invalid primary key. That said,

For gorp I think it is acceptable to have to specify a max string length if you want to index the field.

Seems right.

[pierreprinetti]

PR #247

[FilipDeVos]

Thanks for doing this. Works much better for me ;)

[pierreprinetti]

Thank YOU :+1: