Question

I am working with some queries written by someone else who (in my mind) has gone a little overboard wrapping absolutely everything in calls to Convert().

These queries get data from a source database, and a C# program inserts that data (using System.Data.SqlClient.SqlBulkCopy) into tables in a target database.

So, here is an abbreviated example of the type of thing I'm seeing:

Select
  Convert(nvarchar(50), t.Address1) as Address1
, Convert(smallint, case t.language_cd
                        when 'E' then 31
                        when 'N' then 32
                        when 'D' then 38
                                 else 39
                    end) as LanguageID
from MyTable t

The data types that it's converting to match the data types on the target table (So Address1 on the target table is an nvarchar(50) and LanguageID is a smallint). The source column MyTable.Address1 is a varchar(60) and for LanguageID you can see that we're converting the literals 31, 32, 38, and 39 into smallint.

My question is: Is there any actual reason or advantage to explicitly converting every single column to the data type that matches the target column? Sure, converting a varchar(60) to an nvarchar(50) might fail. But wrapping it in convert() doesn't stop it from failing. I guess one plus would be that if a failure does occur, it will be easy for someone to see at a glance (in the query file) exactly which conversion we were trying to do. But to me that doesn't seem to justify all this clutter in the query file, especially ugly stuff like embedding a CASE statement in a Convert() call. Plus, if we were ever to update the target database to lengthen the column to 60 characters (probably a good idea but not feasible in the short term), then we'd have a mismatch between the Convert() call and the actual definition of the target column.

So, am I missing some reason, some benefit that I get from the explicit conversion that I'm not aware of? Or is my impulse to strip this query down and let the conversion happen implicitly a good one?

Was it helpful?

Solution

Microsoft recommends that when using SqlBulkCopy, the source and target column types match. And according to the internet, SqlBulkCopy isn't as forgiving about mismatched types as INSERT .. SELECT. So I'd say converting in the query is a good idea.

Not to mention that type conversions aren't always straightforward or consistent. For example, which number conversions round and which ones truncate, and are all the conversion rules the same within SQL Server (which is where the CONVERTs are performed, or where conversions would happen if this were INSERT .. SELECT) as they are for SqlBulkCopy (which isn't the same as INSERT .. SELECT)? If you don't know exactly what the rules are, it's wise to be cautious.

Additionally, if the type conversion fails within the query, you may get a more useful error message than if the bulk insert fails. If you want these queries to be more manageable, you could write them like this:

SELECT TOP (0)
  CAST('' AS NVARCHAR(50)) AS Address1,
  CAST(0 AS SMALLINT) AS LanguageID
UNION ALL
SELECT
  t.Address1,
  CASE t.language_cd
    WHEN 'E' THEN 31
    WHEN 'N' THEN 32
    WHEN 'D' THEN 38
  ELSE 39 END AS LanguageID
FROM MyTable t

There's a good chance this rewriting would have no effect on query optimization (if that's an issue, check), and while TOP isn't standard SQL, the code was T-SQL-specific from the CONVERT instead of CAST, so it wasn't standard to begin with.

The advantage here is that the guts of the query isn't cluttered, and the part of the query that announces and enforces the target table column types is completely separate, making it easy to change if a column type is modified.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top