June 21, 2015 at 8:29 am
Hello SQL experts - I'm a newbie. Trying to figure out what is wrong with this code. I think the problem is related to this section but not sure what it is: update employee, work_summary, pay_group
set work_summary.wrks_submitted='Y'
WHERE employee.emp_id = work_summary.emp_id
Probably some glaring problem where my newbie-ness is obvious!
SELECT
employee.EMP_NAME
, work_summary.wrks_submitted
, employee.emp_fullname
, pay_group.paygrp_end_date as "Period Ending"
, work_summary.wrks_work_date
, employee.EMP_VAL10 AS "Leader Email"
, employee.EMP_VAL15 AS "Employee Email"
, employee.EMP_HIRE_DATE as "Hire Date"
, employee.EMP_STATUS as "Status"
, employee.EMP_VAL2 as "Business Area"
, employee.EMP_VAL5 as "Loc"
, employee.EMP_VAL9 as "EmpType"
, employee.EMP_VAL14 as "Auto-Paid"
, employee.EMP_VAL8 as "CoCode"
FROM employee, work_summary, pay_group
WHERE employee.emp_id = work_summary.emp_id
AND pay_group.paygrp_id = employee.paygrp_id
AND (employee.emp_val8 = 'CSC' or employee.emp_val8 = 'CTC')
AND work_summary.wrks_work_date BETWEEN pay_group.paygrp_start_date and pay_group.paygrp_end_date
AND employee.calcgrp_ID in ('10001', '10002', '10003', '10004', '10005', '10006', '10007') AND employee.emp_name = '999999980'
update employee, work_summary, pay_group
set work_summary.wrks_submitted='Y'
WHERE employee.emp_id = work_summary.emp_id
AND pay_group.paygrp_id = employee.paygrp_id
AND (employee.emp_val8 = 'CSC' or employee.emp_val8 = 'CTC')
AND work_summary.wrks_work_date BETWEEN pay_group.paygrp_start_date and pay_group.paygrp_end_date
AND employee.calcgrp_ID in ('10001', '10002', '10003', '10004', '10005', '10006', '10007') AND employee.emp_name = '999999980'
Sandy Tucker
June 21, 2015 at 9:19 am
Please disregard, I think I solved my own problem using the below at the beginning of my update/set area. Thanks!
update work_summary
set work_summary.wrks_submitted='Y'
FROM employee, work_summary, pay_group
Sandy Tucker
June 21, 2015 at 9:47 am
Sandy,
There's a ton of danger in joined updates unless things are done just right and I see a couple of possible "newbie" problems in your examples. Some of these problems can cause bad updates and some can cause a normally well behaved update that runs in just a second or two to slam a half dozen CPUs into the wall for a couple of hours. If you'd post your full UPDATE code, I'd be happy to check it for you and explain anything I see wrong.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 21, 2015 at 10:42 am
Hi Jeff! That's very generous of you. I would love to have you take a look. I only tested this against one record so definitely interested in bad structure that may be a problem when running against thousands of records! Here's the full code I ran successfully against one record:
SELECT
employee.EMP_NAME
, work_summary.wrks_submitted
, employee.emp_fullname
, pay_group.paygrp_end_date as "Period Ending"
, work_summary.wrks_work_date
, employee.EMP_VAL10 AS "Leader Email"
, employee.EMP_VAL15 AS "Employee Email"
, employee.EMP_HIRE_DATE as "Hire Date"
, employee.EMP_STATUS as "Status"
, employee.EMP_VAL2 as "Business Area"
, employee.EMP_VAL5 as "Loc"
, employee.EMP_VAL9 as "EmpType"
, employee.EMP_VAL14 as "Auto-Paid"
, employee.EMP_VAL8 as "CoCode"
FROM employee, work_summary, pay_group
WHERE employee.emp_id = work_summary.emp_id
AND pay_group.paygrp_id = employee.paygrp_id
AND (employee.emp_val8 = 'CSC' or employee.emp_val8 = 'CTC')
AND work_summary.wrks_work_date BETWEEN pay_group.paygrp_start_date and pay_group.paygrp_end_date
AND employee.calcgrp_ID in ('10001', '10002', '10003', '10004', '10005', '10006', '10007') AND employee.emp_name = '999999980'
update work_summary
set work_summary.wrks_submitted='Y'
FROM employee, work_summary, pay_group
WHERE employee.emp_id = work_summary.emp_id
AND pay_group.paygrp_id = employee.paygrp_id
AND (employee.emp_val8 = 'CSC' or employee.emp_val8 = 'CTC')
AND work_summary.wrks_work_date BETWEEN pay_group.paygrp_start_date and pay_group.paygrp_end_date
AND employee.calcgrp_ID in ('10001', '10002', '10003', '10004', '10005', '10006', '10007') AND employee.emp_name = '999999980'
Sandy Tucker
June 21, 2015 at 3:46 pm
Hi Sandy,
Here's how I would write the UPDATE statement if it were me.
UPDATE ws
SET ws.wrks_submitted = 'Y'
FROM dbo.employee e
JOIN work_summary ws ON e.emp_id = ws.emp_id
JOIN pay_group pg ON e.paygrp_id = pg.paygrp_id
WHERE e.emp_name = '999999980'
AND e.emp_val8 IN ('CSC','CTC')
AND e.calcgrp_ID IN ('10001','10002','10003','10004','10005','10006','10007')
AND ws.wrks_work_date BETWEEN pg.paygrp_start_date AND pg.paygrp_end_date
;
Here are the changes I made and my reasons why. Of course, these are just suggestions from an old dude that's run into a whole bunch of problems in the last 2 decades on SQL Server. Similar conventions can be applied to the SELECT you posted, as well.
1. I changed all the table names from a 1 part naming convention to a 2 part naming convention for performance and safety. It would take a while to explain all the reasons but (and I've never confirmed it but seems like the right thing to me) if you have a 1 part naming convention, SQL server will first look for the table under your schema if there is one. If it doesn't find it there, it checks in the "master" database. If it doesn't find it there, then it finally looks in the "dbo" schema of the current database. All of that takes extra time which can really hurt code that's hit on a lot. It's also safer to use the 2 part naming convention because if it actually does only live under the schema of the user that created it (using 1 part naming in the process), then most other users won't be able to use the code. That might be what's expected but, even then, it should be the 2 part naming convention just in case someone makes another identically named table under "dbo" or in the "master" database. It just easier to remember to always use 2 part naming rather than trying to remember when it's ok not to.
2. I moved all the table joining criteria out of the WHERE clause (old "equi-join" method) to the "standard" ANSI join method. This allows for some obvious separation between what is join criteria (Join/On) compared to what is column filter criteria (in the WHERE clause). I say "standard" in quotes because you usually have to use the old "equi-join" method in correlated subqueries and APPLY clauses.
3. I added a 1 or two character table alias (ws, e, pg) to the tables in the FROM clause and use them to replace all the table name references for the two part column names. Some will disagree but, especially for such short code, it removes a lot of visual clutter and makes it easier to physically align the code for easier reading. Of course, the aliases should mean something (usually, abbreviations for the related tables).
4. I changed the UPDATE clause to use the correct alias instead of a table name for performance and safety. This method worked long before it was ever documented. I've not done an actual check for performance on it in over a decade but it did (and I suspect, still does) provide a bit of a performance increase especially on GUI code that's hit a lot. It also prevents the mistake of leaving out the target table of the UPDATE from the FROM clause, which is the primary cause of the huge performance problem I mentioned before. People that used the tablename.columnname format for columns used in Joins ran into this problem a lot... most never knew what the problem was, though and just suffered the performance problem and justified it with "well, there is a lot going on there'. Of course, that's the worst thing to do.
5. I changed an embedded OR to an IN in the WHERE clause just to reduce a little more visual clutter. If the calcgrp_ID column in the Employee table is actually numeric instead of character based, you might be able to do something similar using BETWEEN 10001 and 10007 instead of the 7 part IN currently being used. And, if it IS a numeric column, it would also save on some implicit conversions. I also put all the criteria for the Employee table in one spot just for readability during future modifications or troubleshooting.
6. Last and certainly not least, I applied my standard formatting conventions. All SQL Keyword in upper-case. All schema names and table aliases in lower-case. I would have changed all the table and column names to be initial-caps for each word in the name, but that would rubber-hose you if your database is case sensitive. Then, I applied my "river" format for indenting and did some vertical alignment of things like table alias names and "=" signs, which not only provides this old dude with some easy reading, but also gives me the opportunity to do a quick review of the code to double check for mistakes.
And, no... your code didn't have the performance problem I thought it might because you did, in fact, include the target table of the joined UPDATE in the FROM clause.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 22, 2015 at 6:17 am
Wow Jeff thank you so much for all your valuable insight! I had started with something a little more like this as far as assigning a shortened value to table names, but something must have been off because I was having a hard time running it successfully. I assume I need to go through the rest of the code and replace the table names to “e”, “ws” or “pg”? The first from statement, do I need to modify that to add the table abbreviations?
Sandy Tucker
June 22, 2015 at 6:49 am
Why do you select data first and then update?
If you want details of rows updated add OUTPUT clause to the update statement
Far away is close at hand in the images of elsewhere.
Anon.
June 22, 2015 at 3:04 pm
Well it would seem I thought this code was working. But what is happening is that after it runs, the update occurs. But the change isn't permanent. Some time later, the update disappears. Do I need to use the "commit" command? If one was taking this action manually in the record, they would have to click on a "submit" button in order to save the update.
David - thanks for that tip about "output". Yes I wrote it that way I did so I could get results - I'm a "grasshopper" so have not used the output command. I will give that a try but need to get the code working first.
Sandy Tucker
June 22, 2015 at 6:20 pm
SandyTucker (6/22/2015)
Wow Jeff thank you so much for all your valuable insight! I had started with something a little more like this as far as assigning a shortened value to table names, but something must have been off because I was having a hard time running it successfully. I assume I need to go through the rest of the code and replace the table names to “e”, “ws” or “pg”? The first from statement, do I need to modify that to add the table abbreviations?
If you're talking about the "first from statement" as the FROM that you have in your SELECT, then yes. If that's not it, please explain. You're not talking about the fact than a joined UPDATE actually has 2 FROM clauses, one implicit and one explicit, are you?
--Jeff Moden
Change is inevitable... Change for the better is not.
June 22, 2015 at 6:30 pm
SandyTucker (6/22/2015)
Well it would seem I thought this code was working. But what is happening is that after it runs, the update occurs. But the change isn't permanent. Some time later, the update disappears. [font="Arial Black"]Do I need to use the "commit" command? [/font] If one was taking this action manually in the record, they would have to click on a "submit" button in order to save the update.
Not unless the code is in a transaction that starts with a BEGIN TRANSACTION or the server is setup to use explicit transactions.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply