September 20, 2016 at 9:05 am
Based on this below SP.How to know this sp is updating or inserting into a specific table?
Do we need to specify table name here?
CREATE PROCEDURE [dbo].[sampleLog_Insert]
@table varchar(30),
@date varchar(30),
@file varchar(255)
AS
BEGIN
SET QUOTED_IDENTIFIER OFF
declare @query nvarchar(500)
Set @query='UPDATE ' + @table + ' SET DateTimeFile="'+@date+'",NameFile="'+@File+'" Where NameFile is Null'
exec sp_executesql @query
RETURN
END
GO
September 20, 2016 at 9:21 am
The table name is specified as a parameter
@table varchar(30)
That is horrible design and a security nightmare. Completely vulnerable to SQL Injection since no checks are done on the parameters. Terrible code.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
September 20, 2016 at 9:33 am
mcfarlandparkway (9/20/2016)
Based on this below SP.How to know this sp is updating or inserting into a specific table?Do we need to specify table name here?
CREATE PROCEDURE [dbo].[sampleLog_Insert]
@table varchar(30),
@date varchar(30),
@file varchar(255)
AS
BEGIN
SET QUOTED_IDENTIFIER OFF
declare @query nvarchar(500)
Set @query='UPDATE ' + @table + ' SET DateTimeFile="'+@date+'",NameFile="'+@File+'" Where NameFile is Null'
exec sp_executesql @query
RETURN
END
GO
I agree 100% with Gail. Terrible code!
How can you tell if the query is Updating or Inserting into a specific table? The dynamic SQL is only written to handle updates. Update never inserts new values into a table. The only way the posted code will do an insert is if it falls victim to a SQL Injection attack.
For best practices on asking questions, please read the following article: Forum Etiquette: How to post data/code on a forum to get the best help[/url]
September 20, 2016 at 1:12 pm
I completely agree.This is very horrible code. which was written by old people.
Even I wonder after seeing this code..
September 20, 2016 at 2:57 pm
mcfarlandparkway (9/20/2016)
I completely agree.This is very horrible code. which was written by old people.Even I wonder after seeing this code..
Old people should not be allowed to write SQL code...:-D
September 20, 2016 at 3:34 pm
Criticism of poor code and old people aside, the only way to know what table is being referenced is to run a trace (Profiler or background) to capture the incoming parameters. RPC Start would do it.
As already mentioned it must be doing an UPDATE as that is all that can be generated from this code baring SQL Injection.
And yes, it is terrible code, but actually some of us old people aren't the problem. I'm seeing a lot of code like this on client sites being written by youngsters with "T-SQL For Dummies" or "T-SQL in 21 Days" under the belt who think they know how to write code. If I saw a developer doing this he'd be two steps short of a DCM.
My favourite XKCD for DBAs https://www.xkcd.com/327/
Please don't flame us old people.
Leo
22+ years in MS-SQL Server and not a spring chicken when I got into it.
Leo
Nothing in life is ever so complicated that with a little work it can't be made more complicated.
September 20, 2016 at 5:40 pm
mcfarlandparkway (9/20/2016)
I completely agree.This is very horrible code. which was written by old people.Even I wonder after seeing this code..
Santa is w-a-t-c-h-i-n-g! 😉 Us "old people" wouldn't write code in such a manner.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 20, 2016 at 5:42 pm
John Rowan (9/20/2016)
mcfarlandparkway (9/20/2016)
I completely agree.This is very horrible code. which was written by old people.Even I wonder after seeing this code..
Old people should not be allowed to write SQL code...:-D
Ok... how would everyone like to tell their mama's that some old dude just kicked their a$$? 😛
--Jeff Moden
Change is inevitable... Change for the better is not.
September 21, 2016 at 7:34 am
Jeff Moden (9/20/2016)
John Rowan (9/20/2016)
mcfarlandparkway (9/20/2016)
I completely agree.This is very horrible code. which was written by old people.Even I wonder after seeing this code..
Old people should not be allowed to write SQL code...:-D
Ok... how would everyone like to tell their mama's that some old dude just kicked their a$$? 😛
Agreed! I'm not 'old', but in in our industry, I sure feel it. I can run circles around most of these young bucks.
September 21, 2016 at 8:47 am
As mentioned, this is an horrible design.
If you have to live with an app which uses it, I would alter it so the parameters are logged and a check made that the table exists.
(You should also add exception handling etc)
In outline:
-- Create logging table
-- Something like
--CREATE TABLE dbo.SampleLogInsertParams
--(
--Id int IDENTITY NOT NULL
--CONSTRAINT PK_SampleLogInsertParams PRIMARY KEY
--,InsertTime datetime NOT NULL
--CONSTRAINT DF_InsertTime DEFAULT(CURRENT_TIMESTAMP)
--,PTable sysname NOT NULL
--,PDate date NOT NULL
--,PFile varchar(255) NOT NULL
--);
SET ANSI_NULLS, QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[sampleLog_Insert]
@table sysname,
@date date,
@file varchar(255)
AS
SET NOCOUNT ON;
DECLARE @query nvarchar(500);
INSERT INTO dbo.SampleLogInsertParams(PTable, PDate, PFile)
VALUES(@table, @date, @file);
-- alter if more than one schema
SELECT @query = N'UPDATE ' + TABLE_SCHEMA + N'.' + TABLE_NAME + N' SET DateTimeFile = @date, NameFile = @file WHERE NameFile IS NULL'
FROM INFORMATION_SCHEMA.TABLES
WHERE TABLE_NAME = @table;
EXEC sp_executesql @query, N'@date date, @file varchar(255)', @date, @file;
GO
September 21, 2016 at 1:38 pm
Ken McKelvey (9/21/2016)
As mentioned, this is an horrible design.If you have to live with an app which uses it, I would alter it so the parameters are logged and a check made that the table exists.
(You should also add exception handling etc)
In outline:
-- Create logging table
-- Something like
--CREATE TABLE dbo.SampleLogInsertParams
--(
--Id int IDENTITY NOT NULL
--CONSTRAINT PK_SampleLogInsertParams PRIMARY KEY
--,InsertTime datetime NOT NULL
--CONSTRAINT DF_InsertTime DEFAULT(CURRENT_TIMESTAMP)
--,PTable sysname NOT NULL
--,PDate date NOT NULL
--,PFile varchar(255) NOT NULL
--);
SET ANSI_NULLS, QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[sampleLog_Insert]
@table sysname,
@date date,
@file varchar(255)
AS
SET NOCOUNT ON;
DECLARE @query nvarchar(500);
INSERT INTO dbo.SampleLogInsertParams(PTable, PDate, PFile)
VALUES(@table, @date, @file);
-- alter if more than one schema
SELECT @query = N'UPDATE ' + TABLE_SCHEMA + N'.' + TABLE_NAME + N' SET DateTimeFile = @date, NameFile = @file WHERE NameFile IS NULL'
FROM INFORMATION_SCHEMA.TABLES
WHERE TABLE_NAME = @table;
EXEC sp_executesql @query, N'@date date, @file varchar(255)', @date, @file;
GO
MUCH safer.
I'm also thinking that the OP should include the schema name in the input parameters, as well.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 21, 2016 at 2:17 pm
Ken McKelvey (9/21/2016)
As mentioned, this is an horrible design.If you have to live with an app which uses it, I would alter it so the parameters are logged and a check made that the table exists.
(You should also add exception handling etc)
As a side note. If you're logging the parameters, you should also log the user and a timestamp for future reference, IMHO.
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply