In the last few weeks at work, I have had multiple people have issues using a parameterized query in .NET that involved a temp table. It took a little bit of digging, but we finally tracked down the issue. This post is going to cover the cause of the issue as well as a couple of ways to fix it. The database used in this post is Wide World Importers sample database from Microsoft. For instructions on setting it up check out my Getting a Sample SQL Server Database post from last week.
Sample Project Creation
To keep things as simple as possible I am using a console application created using the following .NET CLI command.
dotnet new console
Followed by this command to add in the SQL Client from Nuget.
dotnet add package System.Data.SqlClient
The following is the full Program class with the sample code that will result in the exception this post is dealing with. Yes, I am aware this isn’t the be way to structure this type of code so please don’t judge it from that aspect. It is meant to be simple to demonstrate the issue.
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Running sample");
using (var connection =
new SqlConnection(@"Data Source=YourServer;
Initial Catalog=YourDatabase;
Integrated Security=SSPI;"))
{
connection.Open();
using (var command = connection.CreateCommand())
{
SqlTest(command);
}
}
Console.ReadLine();
}
private static void SqlTest(SqlCommand command)
{
command.CommandText = @"SELECT OrderId
,CustomerId
,SalespersonPersonID
,BackorderOrderId
,OrderDate
INTO #backorders
FROM Sales.Orders
WHERE BackorderOrderID IS NOT NULL
AND OrderDate > @OrderDateFilter";
command.Parameters.Add("@OrderDateFilter",
SqlDbType.DateTime)
.Value = DateTime.Now.AddYears(-1);
command.ExecuteNonQuery();
command.CommandText = "SELECT OrderId FROM #backorders";
using (var reader = command.ExecuteReader())
{
while (reader.Read())
{
Console.WriteLine(reader["OrderId"]);
}
}
}
}
The Error
Running the application as it exists above will result in the following error.
Invalid object name ‘#backorders’.
Strange error since we just created the #backorder temp table. Let’s give it a try without the filter. The query now looks like the following.
command.CommandText = @"SELECT OrderId
,CustomerId
,SalespersonPersonID
,BackorderOrderId
,OrderDate
INTO #backorders
FROM Sales.Orders
WHERE BackorderOrderID IS NOT NULL";
command.ExecuteNonQuery();
Now the application runs without any issues. What if we try adding back the filter, but without using the command parameter?
command.CommandText = @"SELECT OrderId
,CustomerId
,SalespersonPersonID
,BackorderOrderId
,OrderDate
INTO #backorders
FROM Sales.Orders
WHERE BackorderOrderID IS NOT NULL
AND OrderDate > '2018-01-01'";
command.ExecuteNonQuery();
Again the application runs without any issues.
The Reason
Why is it that adding a command parameter is causing our temp table to disappear? I discovered the issue by using SQL Server Profiler (in SQL Server Management Studio it can be found in Tools > SQL Server Profiler). With the code back to the original version with the command parameter and Profiler connected to the same server as the sample application running the sample application shows the following command received by SQL Server.
exec sp_executesql N'SELECT OrderId
FROM #backorders',
N'@OrderDateFilter datetime',
@OrderDateFilter='2017-08-28 06:41:37.457'
It turns out that when you use command parameters in .NET it gets executed on SQL Server using the sp_executesql stored procedure. This was the key bit of information I was missing before. Now that I know parameterized queries are executed in the scope of a stored procedure it also means the temp table used in our first query is limited to the usage within the stored procedure in which it was created.
Options to Fix
The first option is to not use parameters on your initial data pull. I don’t recommend this option. Parameters provide a level of protection that we don’t want to lose.
The second option and the way we addressed this issue is to create the temp table first. Now that the temp table has been created outside of a stored procedure it is scoped to the connection and then allows us to insert the data using parameters. The following code is our sample using this strategy.
command.CommandText = @"CREATE TABLE #backorders
(
OrderId int
,CustomerId int
,SalespersonPersonID int
,BackorderOrderID int
,OrderDate date
)";
command.ExecuteNonQuery();
command.CommandText = @"INSERT INTO #backorders
SELECT OrderId
,CustomerId
,SalespersonPersonID
,BackorderOrderId
,OrderDate
FROM Sales.Orders
WHERE BackorderOrderID IS NOT NULL
AND OrderDate > @OrderDateFilter";
command.Parameters.Add("@OrderDateFilter",
SqlDbType.DateTime)
.Value = DateTime.Now.AddYears(-1);
command.ExecuteNonQuery();
Wrapping Up
I hope this saves someone some time. Once I understood what was going on the issue made sense. How .NET deals with a SQL command with parameters is one of those things that just always worked and I never had the need to dig into until now. Always something new to learn which is one of the reasons I love what I do.
Also published on Medium.
Sql injection is a bad practice. Why not call a strore procedure with a parameter list and just return the dataset instead of storing in a temp table to execute another query to retrieve the result. I admit your example is a good illustration on how sql server behaves in different scenarios. However your recommendation for a fix would not hold in many shops in a code review.
There are many different options and different shops have different preferences for how this kind of situation should be handled. Using SQL in code with SQL parameters on the .NET side protects against SQL injection just as well as stored procs.
Surely you now have 1 table used by multiple users. How do you handle this concurrency?
If you are referring to the temp table its scope is narrow enough (at largest scope a single connection) that concurrency isn’t a problem.
Why create the temp table at all? As I read through your code, after wondering why you were creating the temp table, I immediately wondered if it was going out of scope on you. But in addition, I expected that the error was going to be because you were opening a reader against a statement that doesn’t return any data. You didn’t mention the fact that your “solution” queries use ExecuteNonQuery while your “problem” code uses ExecuteReader. That’s probably important also. (I did have another theory that you needed to qualify your column names, which is a best practice, since both #backorders and Sales.Orders have common column names).
In short, I think there was more than one issue with the code. Glad you got it sorted, though! SQL Profiler is a great call for something like this, although you might want to start using Extended Events instead, since they can get the same information with less performance impact than Profiler traces sometimes have.
This is a contrived example and not a real use case just enough to show the issue. As for the ExecuteReader vs ExecuteNonQuery both would fail if the temp table was created using a call with parameters because it would be out of scope.
This wasn’t meant to be an example of best practices, instead, I was trying to show an issue and some options to fix that issue. I haven’t looked at Extended Events thanks for the tip!
That’s fair, but I think the code should be refined a little bit so that it shows the issue you are illustrating, but not others. That was my point – your original code may have had more than one problem and that might make the post confusing to a newbie (I’m definitely an “oldbie”, btw … using MSSQL since version 4.21a). The point you are making is that you can’t ship a statement to SQL Server that references a temp table that will not exist.
It does look like I might have misread your code yesterday – I don’t see you trying to SELECT with ExecuteNonQuery, or running INSERT with ExecuteReader. Maybe you tidied that bit up. If not, I’m not going to triple-check myself, b/c I still have to eat dinner! I’ll just say sorry if I misread that initially.
The only thing I see now is that if you would have dropped into a debugger or run Profiler before trying to repro the problem, I think you would have found that it was the SELECT that returned an error. However, you say effectively “hey look, it works if I get rid of the filter” when the reason those repro attempts did not yield errors is that you didn’t run the statement that caused the error (the ExecuteQuery).
Not trying to nitpick. I wouldn’t bother if I didn’t think that it’s a post worth posting, and a post worth posting is worth posting as well as possible, since posts online have an eternal lifespan, thanks to Google and the Wayback Machine.
One thing that interests me is whether the connection was reset in the scenario that caused the error originally. I am wondering about this because if you ran the SELECT INTO in a database utility like SSMS and kept the connection open, that #backorders table would stick around until you dropped it or you disconnected. I wonder if ExecuteQuery disconnects implicitly, allowing SQL Server to drop the temp table.
Cheers,
Chris
Did you try with a table variable vs temp table?
@ vs #
Hey Jay! I didn’t try a table variable. Good thought and something to try for sure.
Hey Jay, just FYI:
@table variables are even more restricted in scope than #temp tables. A #temp table can survive from one batch to the next and will remain as long as the connection that created it is still connected unless it is explicitly dropped. @table variables cannot survive from one batch to the next.
You comment about a temp table having the scope of a stored procedure is wrong. Temp tables are connection scoped and will exist until they are dropped or the connection is closed.
You could also use a global temp table, but none of what you are doing in this post is a particularly good approach, from a data tier perspective. As someone who has seen this type of code professionally, doing anything like this for production code is asking for trouble. I realize you are trying to show a specific scenario, but this example is going to lead people astray.
I was under the same impression about them being connection scoped which is the whole reason for this post.
Global temp tables are dangerous and not the way I would fix this issue.
Yes, global temp tables are a terrible idea, but so is not just calling a stored procedure here.
I suspect connection pooling may be at issue here. See what happens if you disable it. Temp tables are definitely connection scoped, so something else is at play.
This has nothing to do with connection pooling it is all about the fact that once a command has parameters attached it is executed via a stored proc which changes the scope of the temp table.
No, if you use the same connection, the temp table will exist, therefore you aren’t using the same connection.
Try it in the situation outlined in the post you will be surprised just as I was.
You’re right, the sp_executesql runs on a separate connection, and your temp table is there. Actually, what happens if you use a reader on you last call? I’ll wager you get zero records because you’ve really created two different temp tables with the same name. I’ll test myself later. No error, but not what you want either.
Using a temp table across calls to a database is not a good practice. There really isn’t much you couldn’t do in a single call.
That is the tricky part of the problem it isn’t a different connection.
If you read the MSDN article on sp_executesql, it states that no local cursors or variables can be referenced by a call to sp_executesql, and vice versa. I don’t see it say specificly that the connection is different, but it behaves that way. I’m 99% sure you created two tables, one empty.
On the example with the problem it throws an exception since the temp table no longer exists.
Your code does appear to fill the temp table, but I’m still not sure why it works. If you use Profiler and pull the executed SQL, running it all as one batch in SSMS returns the expected zero records. The parameterized query actually runs as an RPC from the app tier, which must be the significant difference.
I think the lack of clarity is just another reason why you want to find a way around using temp tables across calls. Microsoft could also change this behavior at any time, intentionally or not, and break your code.
We are miss communicating in some way. If you run the examples hopefully it will be more clear.
Robin is correct. I made a similar comment before I saw theirs. Trying this without connection pooling is a good idea.
I want to call “Mulligan” on my last post. I wasn’t thinking about the fact that sp_executesql creates a child connection that does not inherit the parent’s #temp tables. Robin was correct that it is using another connection (technically an execution context – it actually has the same SPID … or so it appears based on a very quick test.
Our solution to this is usually something like this, but we do it on the DB side, in a stored procedure.
INSERT INTO #table
EXEC sp_executesql ;
But for doing this in .Net, we always have stored procedures to call, which avoids all sorts of problems and also gives us excellent performance monitoring and tuning advantages.
Now that I’m finally on the same page with you, I have a question. Honest, I do. But I forgot what it is. I’ll post once more if I can remember it.
And any post that makes me think is a good post. Keep it up, Eric.
Creating a temp table in one command and using it another is a bad idea whatever the use case it is. Your sample used just sql client, what if you decide to use entity framework? You cannot even guarantee that you will use same connection in these two commands. If you really have to use this, consider merging insert and select into same statement.
It is all using the same command. This is specifically for when using the SQL Client with a case that can’t be executed in a single statement.
Again not saying any thing about what is best this is just an issue that some one could hit and one option to solve.
Feels like these comments are missing the mark on the goal of this article which is quite sad as this is an excellent writeup on a real world issue.
Great work Eric.
Thanks Joe! Here is to hoping that the comments have added some clarity.
My main complaint is still why would anyone ever need to do this? Just make it one call, and it is never an issue. I can’t think of one case where a problem that can be solved with a temp table across calls couldn’t be solved in better way.
Robin,
I think it entirely depends on the situation and could be influenced by a number of factors. Also, Microsoft could change the behavior of anything at anytime, intentionally or not, and break everybody’s code.
It is true Microsoft can do what they want, but they try not to break features people use. I have no doubt Microsoft would consider this an anti-pattern, which probably doesn’t warrant much protection.
I’ve tried running this code different ways and the results differ depending on subtle changes, several the post shows. This code is brittle, and using temp tables this way is difficult to maintain when placed in larger code bases. I’ve seen similar code to this in many products, and it has always made it harder to debug. This is a very interesting edge case to examine, but I don’t think you’ll find many savy developers/DBAs who would allow this in production code.
Robin,
I’m going to have to disagree. I think the most savvy developers know when its necessary to get their hands dirty and how to safely and effectively do so. All in moderation, dear Robin.
If something simple appears complex, you’re doing it wrong, Cowboy Joe.
Pingback: Professional Development 10/08/2018 – 10/14/2018 – The Software Mentor