0

I have a procedure that can be changed dynamically by user for multi column and I write it in SQL when I run it. Everything is OK in SQL and Server Explorer in Visual Studio but when I want use it in C# and call it, it just return 0 always. Can anybody help me?!

CREATE PROCEDURE [dbo].[PDaynamicActualBy2Column]
    @Colname1 nvarchar(100),
    @VarCol1 nvarchar(100),
    @Colname2 nvarchar(100),
    @VarCol2 nvarchar(100),
    @VarWeekNum nvarchar(100)
as
    DECLARE @temp nvarchar(1500)
    set @temp='SELECT SUM([dbo].[WeekActualTemp].[ACTUAL]) from [dbo].[MAINTB] join [dbo].[WeekActualTemp] on [dbo].[MAINTB].[UQ]=[dbo].[WeekActualTemp].[UQ] 
    where [dbo].[WeekActualTemp].[WeekNO]='+@VarWeekNum+' And [dbo].[MAINTB].'+@Colname1+' = '''+@VarCol1+''' And [dbo].[MAINTB].'+@Colname2+' = '''+@VarCol2+''''
    exec (@temp)
GSerg
  • 76,472
  • 17
  • 159
  • 346
Miad BayaniRad
  • 66
  • 1
  • 11
  • 1
    Little suggestion, stick `PRINT @temp` before your `EXEC`. Then you can debug the sql being executed, maybe something will jump out at you. Heck, why not print the sql being executed and post alongside your question...that would be cool. – Simon Wilson Dec 08 '19 at 10:35
  • 6
    This procedure is a security hazard as it's an open door for SQL Injection attacks. Also, using more than 2 parts in column identifiers is a deprecated - and even if it wasn't - there are [good reasons](https://stackoverflow.com/a/58607082/3094533) why you should stick with 2-parts identifiers for your columns. Read my blog post entitled [The do’s and don’ts of dynamic SQL for SQL Server](https://zoharpeled.wordpress.com/2019/09/12/the-does-and-donts-of-dynamic-sql-for-sql-server-%ef%bb%bf/) to learn how to write safe dynamic SQL. – Zohar Peled Dec 08 '19 at 10:35
  • my only way to do some calculate for my column which user select manual is this way, any way i get my way how to solve it but in c# code when call this proceduer it always return 0 – Miad BayaniRad Dec 08 '19 at 10:42
  • 1
    Please [edit] your question to include the c# code that executes the procedure and attempts to get the value back to c#. – Zohar Peled Dec 08 '19 at 10:47
  • Are you using `ExecuteScalar` or `ExecuteNonQuery` from C#? – GSerg Dec 08 '19 at 10:53
  • Are you using EF ? – xwpedram Dec 08 '19 at 10:56
  • i using linq to execute procedure – Miad BayaniRad Dec 08 '19 at 10:59
  • 2
    Again, please [edit] your question to include the c# code that executes the procedure. – Zohar Peled Dec 08 '19 at 13:08

3 Answers3

2

This does not address the problem in the C#, however, it does address the huge injection issue you have in your code. Like mentioned, don't inject your parameters and properly quote your dynamic object names. This results in something like the below:

CREATE PROC dbo.PDaynamicActualBy2Column @Colname1 sysname, @VarCol1 nvarchar(100), @Colname2 sysname, @VarCol2 nvarchar(100), @VarWeekNum int AS --Assumed @VarWeekNum is an int, as why else is it called "num"?
BEGIN

    DECLARE @SQL nvarchar(MAX),
            @CRLF nchar(2) = NCHAR(13) + NCHAR(10);

    SET @SQL = N'SELECT SUM(WAT.Actual) AS ActualSum' + @CRLF +
               N'FROM dbo.MAINTB MTB' + @CRLF +
               N'     JOIN dbo.WeekActualTemp WAT ON MTB.UQ = WAT.UQ' + @CRLF +
               N'WHERE WAT.WeekNO = @VarWeekNum' + @CRLF +
               N'  AND MTD.' + QUOTENAME(@Colname1) + N' = @VarCol1' + @CRLF +
               N'  AND MTD.' + QUOTENAME(@Colname2) + N' = @VarCol2;';

    --PRINT @SQL; Your Best Friend

    EXEC sp_executesql @SQL, N'@VarCol1 nvarchar(100),@VarCol2 nvarchar(100),@VarWeekNum int', @VarCol1, @VarCol2, @VarWeekNum;

END;
GO

Because you're only returning a single scalar value, you could also use an OUTPUT parameter as well, instead of a SELECT to the display the value. This would look like the below:

CREATE PROC dbo.PDaynamicActualBy2Column @Colname1 sysname, @VarCol1 nvarchar(100), @Colname2 sysname, @VarCol2 nvarchar(100), @VarWeekNum int, @ActualSum int OUTPUT AS --Assumes Actual is an int in your table. Use an appropriate data type
BEGIN

    DECLARE @SQL nvarchar(MAX),
            @CRLF nchar(2) = NCHAR(13) + NCHAR(10);

    SET @SQL = N'SELECT @ActualSum = SUM(WAT.Actual)' + @CRLF +
               N'FROM dbo.MAINTB MTB' + @CRLF +
               N'     JOIN dbo.WeekActualTemp WAT ON MTB.UQ = WAT.UQ' + @CRLF +
               N'WHERE WAT.WeekNO = @VarWeekNum' + @CRLF +
               N'  AND MTD.' + QUOTENAME(@Colname1) + N' = @VarCol1' + @CRLF +
               N'  AND MTD.' + QUOTENAME(@Colname2) + N' = @VarCol2;';

    --PRINT @SQL; Your Best Friend

    EXEC sp_executesql @SQL, N'@VarCol1 nvarchar(100),@VarCol2 nvarchar(100),@VarWeekNum int, @ActualSum int OUTPUT', @VarCol1, @VarCol2, @VarWeekNum, @ActualSum OUTPUT; --Again, assumes Actual is an int.

END;
GO

Notice, as mentioned in the comments, I get rid of the 3+ part naming for your columns, and alias your tables instead. I then use those aliases to reference to correct object. I've also put "Your best Friend" in the code, should you need to debug it.

Note: As mentioned in a different answer, zero is likely because the SP is returning 0 to mean success. This is a documented and intentional feature of Stored Procedures:

Unless documented otherwise, all system stored procedures return a value of 0. This indicates success and a nonzero value indicates failure.

As the SP above is likely successful, the RETURN value is 0; to denote success. You shouldn't be looking at the RETURN value, but the dataset, or in the latter example the OUTPUT parameter's value. I am sure there are questions on SO for how to use an OUTPUT parameter in linq.

Thom A
  • 88,727
  • 11
  • 45
  • 75
0

Zero in return value of your stored procedure means it executed successfully.

You should return a value by "RETURN" statement or "SELECT" statement for table in return.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
XAMT
  • 1,515
  • 2
  • 11
  • 31
-1

In LINQ you cannot call an SP that have a meta output that is dynamic, you have to write your SP with "select" output and make a model and then go to SP and edit it again.

Alter PROCEDURE [dbo].[PDaynamicActualBy2Column]
    @Colname1 nvarchar(100),
    @VarCol1 nvarchar(100),
    @Colname2 nvarchar(100),
    @VarCol2 nvarchar(100),
    @VarWeekNum nvarchar(100)
as
    DECLARE @temp nvarchar(1500)
    set @temp='SELECT SUM([dbo].[WeekActualTemp].[ACTUAL]) from [dbo].[MAINTB] join [dbo].[WeekActualTemp] on [dbo].[MAINTB].[UQ]=[dbo].[WeekActualTemp].[UQ] 
    where [dbo].[WeekActualTemp].[WeekNO]='+@VarWeekNum+' And [dbo].[MAINTB].'+@Colname1+' = '''+@VarCol1+''' And [dbo].[MAINTB].'+@Colname2+' = '''+@VarCol2+''''
  --  exec (@temp)

SELECT top 0
SUM([dbo].[WeekActualTemp].[ACTUAL]) as sum
from [dbo].[MAINTB] join [dbo].[WeekActualTemp] on [dbo].[MAINTB].[UQ]=[dbo].[WeekActualTemp].[UQ] 

then import SP in your LINQ then comment "select" and uncomment "exec" .

xwpedram
  • 467
  • 1
  • 8
  • 20
  • 1
    I suspect the reason this is getting downvotes is due to the value that this still suffers from some major injection problems, and also still uses 3+ part naming for columns. Both of which are bad ideas for different reasons. – Thom A Dec 08 '19 at 15:19