Need help in Date time Conversion failed error

Transact-SQL
, ,
1

Hi i am trying to execute the below query but unable to execute.
ALTER proc [dbo].[Sp_UP_tbl_del]
(
@tbl_name nvarchar(50),
@Col_id_Nm nvarchar(50),
@unq nvarchar(20)=null
)
as begin
declare @sqldel NVARCHAR(max)
declare @msg nvarchar(100)
if (@unq is not null )
begin
set @sqldel='update '+@tbl_name+' set IsDel=''1'' UpDates='''+dateadd(hour,(8),getutcdate())+''' where '+@Col_id_Nm+' = '''+@unq+''' '
EXEC (@sqldel)
end
else
begin
Set @msg='Unknown Error'
select @msg as msgs
end
end

Error is below
Conversion failed when converting date and/or time from character string.

2

Use PRINT to see what the @sqldel contains. You've missed a comma (at least).

I never think that this sort of generic SQL Sproc is a good idea. At the very least yours is wide open (as it stands) to SQL injection.

1 Like
3

i placed the comma sill the same issue.
the issue is with part "UpDates=dateadd(hour,(8),getutcdate())"
and this project will run in intranet or local setup hence no problem for sql injection

4

I doubt it. This works fine:

SELECT dateadd(hour,(8),getutcdate())

Like I said put a PRINT statement in there so you can see what your string concatenation is actually resulting in.

You have plenty of potential problems associated with SQL Injection, if not something malicious.

For example an embedded quote will break your SQL and result in a runtime error. You may very well struggle to reproduce problems like that, and if that is the case you wind up with users being unhappy because the APP is unstable.

No caching of query plan for performance either ... you could probably fix that using a parameterised query with sp_ExecuteSQL, although you still have the table and column name substitution it would perform better than EXEC for sure.

And so on.

IME this "generic update any table using dynamic SQL" approach is generally a bad ideal. But up to you.

5

Thank you for the explanation i'll keep this in practice.

6

I looked at your code more closely

Your single quotes are wrong, but I expect that is this forum changing them to "pretty"

It would help if you post code here "formatted" - so that the interfering Markdown <spit> that this site uses doesn't mangle your code sample:

    ```sql
    ... your SQL code here ...
    ```

You are trying to concatenate a DATE to a STRING which is why you are getting that error.

So now (the way you have done it) you have to convert your Date to String.

However, SQL will try to parse your string date. The "method" that SQL uses to parse a string-date will depend on a number of settings ... for example, that includes "The language of the currently connected user". So plenty of scope for dates to be parsed differently comparing DEV & Production or even "now" and "in future" (e.g. you change the SQL Server hardware, and the new server has subtly different settings).

You can avoid that by using a string-date format of "yyyymmdd" which is the only format that SQL will parse unambiguously. If you want the TIME too then you can use "yyyymmdd hh:mm:ss.nnn" or you can use the ISO format "yyyy-mm-ddThh:mm:ss.nnn" which SQL will also treat as UNambiguous. Anything else and all bets are off

Or you could use sp_ExecuteSQL and pass the DATEADD value as a DATE PARAMETER in which case it will be a date at all times (i.e. never converted to String) and the problem goes away.

Or you could just code the DATEADD function into your SQL string itself, rather than passing the value. That would work in this case (but not if the date needed to be based on "some other date parameter/column")

Anyway, all this complexity is another good reason to avoid dynamic SQL as much as possible. Whenever we use it (lets assume "with good reason" :slight_smile: ) it always jacks up the amount of DEV time we need :frowning:

7

Thank you so Much can u check this what am i doing wrong here.

USE master
GO

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

SET ANSI_PADDING ON
GO

CREATE TABLE [dbo].[tbl_type](
[type_id] [int] IDENTITY(1,1) NOT NULL,
[type_name] varchar NULL,
[display_title] nvarchar NULL,
[sort] [int] NULL,
[IsActive] [smallint] NULL,
[IsDel] [bit] NULL,
[UpBy] [int] NULL,
[UpDates] [datetime] NULL,
[CrBy] [int] NULL,
[CrDates] [datetime] NULL CONSTRAINT [DF_tbl_type_CrDates] DEFAULT (dateadd(hour,(8),getutcdate())),
CONSTRAINT [PK_tbl_type] PRIMARY KEY CLUSTERED
(
[type_id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

GO

SET ANSI_PADDING OFF
GO
--End of the table
--insert query
insert into tbl_type (type_name,display_title,sort,IsActive,IsDel,CrBy)values ('abc','ABC',1,1,0,0)

USE master
GO
/****** Object: StoredProcedure [dbo].[Sp_UP_tbl_del] Script Date: 26-11-2017 12:46:28 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER proc [dbo].[Sp_UP_tbl_del]
(
@tbl_name nvarchar(50),
@Col_id_Nm nvarchar(50),
@UpBy Nvarchar(50),
@unq nvarchar(20)=null
)
as begin
declare @sqldel NVARCHAR(max)
declare @msg nvarchar(100)
if (@unq is not null )
begin
set @sqldel='update '+@tbl_name+' set IsDel=''1'',UpBy='''+@UpBy+''', UpDates='''+dateadd(hour,(8),getutcdate())+''' where '+@Col_id_Nm+' = '''+@unq+''' '
EXEC (@sqldel)
end
else
begin
Set @msg='Unknown Error'
select @msg as msgs
end
end

--executing the query
Sp_UP_tbl_del 'tbl_type','type_id','2'

--Testing whith printing it
declare @dates datetime
declare @sql nvarchar(max)
set @dates=dateadd(hour,(8),getutcdate())

print @dates

8

Stupid form won't let me post that as the message is too short ... now it won't be of course ...

9

that's all heading-for-a-fall of mixed-datatype which will cause (really hard to diagnose) runtime errors.

This is going to give you so many headaches downstream ...

10

I recommend you don't EVER use VARCHAR without a size. The defaults can be disastrous (SQL will use a default size of 1 in some instances :frowning:

Why are you adding 8 for the default? Is your current location 8-hours adrift from the time that your company uses? This is going to be a real problem if the DB moves to a server in a different time zone.

Dunno if of interest but we start all our tables at different SEEDs. That means that if we accidentally join THIS_ID and THAT_ID that they do NOT find a match (with the tiny amounts of test data that we use in DEV). If everything starts at 1 then chances are much higher that joining THIS_ID and THAT_ID will (wrongly) make a match.

11

We put

SET NOCOUNT ON

at the top of all SProcs, otherwise the "(1234 row(s) affected)" message can upset some APPs

12

i will surely do as u said

13

The server will never move from that location hence putting 8 hours by default, at least for few years the server location will be the same

14

I would suggest you just use "Local Time".

If you need World Time then use datetimeoffset datatype instead.

The effort to change all the places in your code where you have time-adjustment will be monumental "in a few years time" as the software size grows ...

... if you really do need "world time" I would take care of that now, and not hardcode anything with a time-offset that will not be valid for users from multiple timezones.

15

Thank you so much for your response ill surely keep this in mind.
but how ever i am still stuck with my procedure

16

If its the original error then my answer was earlier:

17

can you please give me sql query code for the same i am stuck here i am a learner stuck here.

18

Well then, sorry, but all the more reason NOT to be using Dynamic SQL like this. Wrong solution to what you think the problem is.

I've isolated the core of your code below.

You need to fix that so that there is no compile error

Once you've done that see what SQL code it generates, and whether that looks correct and does what you want.

If not modify this test-rig until it does.

Once it outputs the right value THEN put that code back into your original code.

declare	@tbl_name nvarchar(50) = 'MyTablename',
	@Col_id_Nm nvarchar(50) = 'MyColumnName',
	@unq nvarchar(20)= 'MyUNQValue'

declare @sqldel NVARCHAR(max)
declare @msg nvarchar(100)

SELECT 'update '+@tbl_name
	+ ' set IsDel=''1'' UpDates='''
-- The result from DATEADD is DATE and not VARCHAR so cannot be appended here
	+ dateadd(hour,(8),getutcdate())
	+ ''' where '
	+ @Col_id_Nm+' = '''+@unq+''' '

As I said above:

"you [will] have to convert your Date to String"
"Or you could just code the DATEADD function into your SQL string itself, rather than passing the value [of the function result]"

19

thank you so much :star_struck: