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 beAlter Procedure dbo.sproc_MDB_ClinicLetters @Clinician varchar(4), @Booked_Date Datetime, @Facility varchar(30)WITH ENCRYPTIONasSelect 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_TimeFrom vw_MDB_PatientDemographics as P Inner Join vw_MDB_ClinicHome CH on P_Intid = CH_Intid Inner Join vw_Staff on CH_Clinician = Staff_CodeWhere CH_Clinician = @ClinicianAnd CH_Booked_Date = @Booked_DateAnd CH_Facility = @FacilityGRANT EXEC ON dbo.sproc_MDB_ClinicLetters to MyRole steveAlright 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 withInvalid column name...Go with the flow & have fun! Else fight the flow |
|
|
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! |
|
|
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.steveAlright 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 : 08:40:15
|
you mean there's another error in there somewhere??Go with the flow & have fun! Else fight the flow |
|
|
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? |
|
|
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 |
|
|
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 30not to mention you can do simply this: select Cast(day(getdate()) as varchar)Go with the flow & have fun! Else fight the flow |
|
|
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 interestedcheerssteveAlright 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. |
|
|
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....... |
|
|
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 |
|
|
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 30not 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 varcharSET @s = 'asdf;lkjasdf;lkjasdf;lkj'PRINT @swhich returned 'a'. I didn't use the actual code that he supplied. So much for my testing career!Aj |
|
|
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 ThankssteveAlright 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. |
|
|
|