Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 Site Related Forums
 The Yak Corral
 Gotcha?

Author  Topic 

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2005-06-30 : 04:08:17
This one just caught me out and I'm really hacked off with myself for not spotting it. Now I know all the guru's here will sit back and smile to themselves and say "that would never happen to me" so I'm going to publicly humiliate myself and give you details in the hope that you can do the same with something simple that caught you out and we can all benefit.

Here is the script that gave the error, can you spot what the error would be


Alter Procedure dbo.sproc_MDB_ClinicLetters @Clinician varchar(4), 
@Booked_Date Datetime,
@Facility varchar(30)
WITH ENCRYPTION
as

Select
CH_Intid,
CH_Clinicno,
CH_Booked_Date,
CH_Booked_Time,
P_PEDNO,
P_Address_1,
P_Address_2,
P_City,
P_PTNO,
P_SALUTATION,
P_State,
P_Zip,

GetDate() As CurrDate,
CH_Clinician,

Case
When Clinic_Scheduler_Groups = 'COU' Then vw_Staff.[Name]+ ', '+'Genetic Counsellor'
Else vw_Staff.[Name]
End As Clinician_Name,

vw_Staff.NAME,
CH_Facility_Location,
CH_Instructions,

DateName(dw,CH_Booked_Date)+', '+Cast(DatePart(d,CH_Booked_Date)as varchar)
+' '+DateName(month,CH_Booked_Date)+' '+Cast(DatePart(yyyy,CH_Booked_Date)as varchar)
+' at ' as Date_Time

From vw_MDB_PatientDemographics as P Inner Join vw_MDB_ClinicHome CH on P_Intid = CH_Intid
Inner Join vw_Staff on CH_Clinician = Staff_Code

Where CH_Clinician = @Clinician
And CH_Booked_Date = @Booked_Date
And CH_Facility = @Facility


GRANT EXEC ON dbo.sproc_MDB_ClinicLetters to MyRole




steve


Alright Brain, you don't like me, and I don't like you. But lets just do this, and I can get back to killing you with beer.

spirit1
Cybernetic Yak Master

11752 Posts

Posted - 2005-06-30 : 06:20:48
i'd have to go with
Invalid column name...


Go with the flow & have fun! Else fight the flow
Go to Top of Page

robvolk
Most Valuable Yak

15732 Posts

Posted - 2005-06-30 : 07:17:22
If you mean you used underscores instead of periods for all of your table aliases, then I found it.

Of course, I would never make that error. I'd make some other error that was even dumber and more embarrassing, so there!
Go to Top of Page

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2005-06-30 : 08:36:58
Actually you've both just made my day.

In answer to rob, it's even dumber than that.


steve

Alright Brain, you don't like me, and I don't like you. But lets just do this, and I can get back to killing you with beer.
Go to Top of Page

spirit1
Cybernetic Yak Master

11752 Posts

Posted - 2005-06-30 : 08:40:15
you mean there's another error in there somewhere??

Go with the flow & have fun! Else fight the flow
Go to Top of Page

AndrewMurphy
Master Smack Fu Yak Hacker

2916 Posts

Posted - 2005-06-30 : 08:44:02
Where the GO statement before the GRANT EXEC?
Is the GRANT EXEC not included within the SP?
Go to Top of Page

ajthepoolman
Constraint Violating Yak Guru

384 Posts

Posted - 2005-06-30 : 09:51:58
You didn't tell your varchars what length to be? That would only return the first character.

Cast(DatePart(d,CH_Booked_Date)as varchar)

Aj


Go to Top of Page

spirit1
Cybernetic Yak Master

11752 Posts

Posted - 2005-06-30 : 10:16:27
acctually it wouldn't:
select Cast(DatePart(d, getdate())as varchar)
returns 30

not to mention you can do simply this:
select Cast(day(getdate()) as varchar)


Go with the flow & have fun! Else fight the flow
Go to Top of Page

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2005-06-30 : 11:32:04
I find this fascinating - must do it again sometime. Well done to AndrewMurphy who got it right.

BTW if you have any constructive criticisms I would be interested

cheers

steve

Alright Brain, you don't like me, and I don't like you. But lets just do this, and I can get back to killing you with beer.
Go to Top of Page

AndrewMurphy
Master Smack Fu Yak Hacker

2916 Posts

Posted - 2005-06-30 : 11:50:42
Yippie....I got something right!!!!
Must go home before I ruin my record.......
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2005-06-30 : 13:22:56
"I'm going to publicly humiliate myself"

OK, I'll give it my best shot:

Use IF EXISTS DELETE + CREATE - (script cannot be used on, say, production database if not already pre-existing)

Variable captialisation of vw_Staff.[Name] (sloppy, will fail on case sensitive DB - anyone use one of those?)

Failure to put [xxx] round "vw_Staff.NAME" ("name" is a reserved word, although it will work OK in this usage, at least up to SQL2K)

Failure to put "dbo." in front of table names in FROM / JOIN (run by a user may cause personal tables to be accessed; will be slower because SQL has to check for absence of personal tables before reverting to dbo)

Table aliases, in FROM / JOIN, not used as prefixes to column names (possible future maintenance issue, obfuscating bug making it harder to find)

No error handling / checking (unreported failure will cause down-stream errors, hard to associate with this code)

No return code (default return code may be unpredictable)

No comments (you're not planning to leave, so modification by your successor is not an issue, right?)

No change history (ditto - I'm sure you can remember what you changed and when and why)

x 9
Go to Top of Page

ajthepoolman
Constraint Violating Yak Guru

384 Posts

Posted - 2005-06-30 : 14:24:02
quote:
Originally posted by spirit1

acctually it wouldn't:
select Cast(DatePart(d, getdate())as varchar)
returns 30

not to mention you can do simply this:
select Cast(day(getdate()) as varchar)


Go with the flow & have fun! Else fight the flow



Ah, I tested it in my QA window using a string.

I used

DECLARE @s varchar
SET @s = 'asdf;lkjasdf;lkjasdf;lkj'
PRINT @s

which returned 'a'. I didn't use the actual code that he supplied. So much for my testing career!

Aj
Go to Top of Page

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2005-07-01 : 04:08:02
Kristen, I think they are all good points apart from the last two - the change history is in a comment at the top which I didn't put in. Secondly if/when I leave this guy stands a fair chance of being my successor at which point it will become his problem

Thanks

steve

Alright Brain, you don't like me, and I don't like you. But lets just do this, and I can get back to killing you with beer.
Go to Top of Page
   

- Advertisement -