-2

I'm having a problem when migrating from SQL Server 2008 R2 to SQL Server 2019. My code

DECLARE @str NVARCHAR(50) = 'all',
        @int TINYINT = 1

DECLARE @tmp TABLE (val nvarchar(MAX))
INSERT INTO @tmp VALUES('123')
INSERT INTO @tmp VALUES('all')

SELECT val
FROM @tmp
WHERE @str = 'ALL' OR @int = val

When using SQL Server 2008 R2, it's fine. The output as expected like below

val
123
all

However, when I migrate to SQL Server 2019, the error like below occurs. Besides, it just happens unusually in 2019.

Msg 245 Level 16 State 1 Line 8 Conversion failed when converting the nvarchar value 'all' to data type int.

As you can see, the second condition OR @int = val happened unexpectedly.

I was wondering if it fails due to any breaking changes related to the order of OR operator or case sensitive ALL vs all in the next SQL Server 2008 R2 version.

Updated

Sorry for my reproduce code make you guys confuse. This is my original code

Aaron Bertrand
  • 272,866
  • 37
  • 466
  • 490
Nguyễn Văn Phong
  • 13,506
  • 17
  • 39
  • 56
  • 1
    The error is telling you the problem here. You seemed to have just been "lucky" before that you didn't get this error. `'all'` is *clearly* not a `tinyint`, and yet it will be implicitly converted to one in the `WHERE`. `@int` should be a `varchar`; you're comparing it to a datatype that can hold up to ~1Billion characters. – Thom A Dec 29 '21 at 11:23
  • Nope. Actually `@tmp` table looks like `[strSplit]` [this](https://dbfiddle.uk/?rdbms=sqlserver_2019&fiddle=779a0aff7852ef748ff2e61683e12fda). So you can ignore the `val nvarchar(MAX)`. I just reproduce to make my code as simple as possible, sir @Larnu – Nguyễn Văn Phong Dec 29 '21 at 11:30
  • The code was flawed in the first place though, @NguyễnVănPhong . Like I said, you were simply lucky before . It was, in essence, a ticking time bomb. Well *very* lucky that it didn't happen before in your production environment. – Thom A Dec 29 '21 at 11:39
  • I totally agree with 2 you guys we shouldn't compare `tinyint with nvarchar(max)`. And my code should look like this `WHERE @str = 'ALL' OR (@str <> 'ALL' AND @int = val)`, right? – Nguyễn Văn Phong Dec 29 '21 at 11:46
  • That won't necessarily avoid the problem either. `@int` should be either *not* be an `tinyint` or the column `val` should be a numberical data type (and thus can't have the value `'all'`, and perhaps `NULL` instead. – Thom A Dec 29 '21 at 11:49
  • But your function, `strSplit`, doesn't return a `int`, it returns `varchar(MAX)`; it's not comparable. (And, to cement it, get rid of that function and use a set based method, or the in built `STRING_SPLIT` function.) – Thom A Dec 29 '21 at 11:52
  • My bad. It should be a value less than tinyint size. I just create the sample data. Because `strSplit` is used in many places. So you should ignore `tinyint, nvarchar(max)`. The root cause maybe comes from `WHERE @str = 'ALL' OR @int = val`. It should be `WHERE @str = 'ALL' OR (@str <> 'ALL' AND @int = val)` to make sure that the second OR should not be done, right? sir @AaronBertrand – Nguyễn Văn Phong Dec 29 '21 at 11:57
  • `ONE MORE TIME: SQL Server does not have to follow your code left to right.` yeah. I just need the article or something like that to make sure that :) – Nguyễn Văn Phong Dec 29 '21 at 12:05
  • 2
    Um, ok. [Here's an answer on this site with good info](https://stackoverflow.com/questions/789231/is-the-sql-where-clause-short-circuit-evaluated), and also [an article](https://www.sqlservercentral.com/articles/understanding-t-sql-expression-short-circuiting), but you could also trust that experts answering your request for help here wouldn't be lying to you. And look, if you're willing to change your code anyway, why not just change it right? – Aaron Bertrand Dec 29 '21 at 12:09
  • Wow. Thank you so much for your support. sir @AaronBertrand & Larnu – Nguyễn Văn Phong Dec 29 '21 at 12:10

1 Answers1

2

You should do two of these three things:

(

  1. Either use DECLARE @int nvarchar(max) = 1

    OR

  2. Use WHERE val = CONVERT(nvarchar(max), @int)

)

AND

  1. Change to using STRING_SPLIT. That looping function is among the least efficient methods you could ever use to split strings, even before native solutions existed. See https://sqlblog.org/split

This db<>fiddle fiddle demonstrates.

And this one shows why WHERE @str = 'ALL' OR (@str <> 'ALL' AND @int = val) is not a solution. These patterns you're choosing only work if @str is always 'all', because they all break when it's anything else. So why have the OR at all?

You keep insisting that SQL Server should obey left to right evaluation, but we keep telling you that is simply not the case.

Here is an article by Bart Duncan at Microsoft, who worked on SQL Server, that you should absolutely read in full before posting any more comments or editing your question further. The critical point, though, is:

You cannot depend on expression evaluation order for things like

WHERE <expr1> OR <expr2>

since the optimizer might choose a plan that evaluates the second predicate before the first one.

Aaron Bertrand
  • 272,866
  • 37
  • 466
  • 490
  • Thanks sir. Whether the order of `OR` operator don't make sure `left to right`? So in the case of `strSplit ('all', ',')` should be fail? – Nguyễn Văn Phong Dec 29 '21 at 12:06
  • Let me explain it in more detail. Just a min sir – Nguyễn Văn Phong Dec 29 '21 at 12:14
  • Herre is [my original code](https://dbfiddle.uk/?rdbms=sqlserver_2019&fiddle=8848cb6a68351a19d2b5c90bcd5863d7). Do you mean SQL Server does not have to follow your code `left to right` principle? So we will got the error when execution plan run the second condition first? – Nguyễn Văn Phong Dec 29 '21 at 12:20
  • And again, it doesn't make sense to have an OR at all if the only possible way this code works is if `@str = 'all'`: https://dbfiddle.uk/?rdbms=sqlserver_2019&fiddle=c30c327270dbf26ad6537c61ec07cd91 – Aaron Bertrand Dec 29 '21 at 12:24
  • My where clause should be like this? `WHERE @str = 'all' OR (@str <> 'all' AND @int in (SELECT val FROM dbo.strSplit(@str,',')))` – Nguyễn Văn Phong Dec 29 '21 at 12:25
  • No, it should not. I've told you two ways to fix your code, and I'm not saying anything more. If you don't want to fix it right, and want to insist on believing SQL Server will always obey left to right in spite of all the evidence I've shown you that this is not the case, I can't help you. – Aaron Bertrand Dec 29 '21 at 12:25
  • Thanks a million, sir. I will change my code. 1. Using correct data type. 2. Using `STRING_SPLIT` feature instead of my defined function `dbo.strSplit` 3. Don't depend on expression evaluation order for things like “WHERE OR “. Anything else sir? – Nguyễn Văn Phong Dec 29 '21 at 12:39