A Better way to write some SQL

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

SQL Server 2000 Forums

SQL Server Development (2000)

A Better way to write some SQL

Author

Topic

macca
Posting Yak Master

146 Posts

Posted - 2005-06-30 : 10:21:52

Would anyone have any suggestions how to better write the below SQL code. I do 3 Selects one after another at the start of the code, I think this is a bit shabby!!!

Anyone any Ideas??

CREATE PROCEDURE sproc_Number
(
@deptID int,
@serOff varchar(4) OUTPUT,
@returnNum int OUTPUT
)

AS

DECLARE @Service char(4)
DECLARE @Office char(4)
DECLARE @Num int
DECLARE @tempNum int
DECLARE @tempNumTwo int
DECLARE @temp varchar(4)

SELECT @Service = Service, @Office = Office FROM Service WHERE DeptID = @deptID

SET @serOff = RTRIM(@Service) + RTRIM(@Office)

SELECT @Num = Num FROM GenNum WHERE @serOff = SO

SET @tempNum = @Num + 1

SELECT @tempNumTwo = Num FROM GenNum WHERE @serOff = SO

IF @tempNumTwo = @TempNum
BEGIN
SET @tempNumTwo = @tempNum + 1
UPDATE GenNum SET Num = @tempNumTwo WHERE SO = @serOff
SET @returnNum = @tempNumTwo
END
ELSE
BEGIN
UPDATE GenNum SET Num = @tempNum WHERE SO = @serOff
SET @returnNum = @tempNum
END

SELECT @returnNum
GO

nathans
Aged Yak Warrior

938 Posts

Posted - 2005-06-30 : 10:58:47

Is SO a column of GenNum?

-- this is not valid

SELECT @Num = Num FROM GenNum WHERE @serOff = SO

Do you mean

WHERE SO = @serOff

Can you please post the create scripts for the involved tables.

And review your logic in the IF statement. You are essentially doing this:

select	@num = num,
	@tempNum = (num + 1),
	@tempNumTwo = num
from genNum 
where SO = @serOff

Then checking to see if @tempNumTwo = @tempNum... num will never equal (num + 1).

Please post a little more detail, follow the guidelines in this link:
[url]http://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspx[/url]

macca
Posting Yak Master

146 Posts

Posted - 2005-06-30 : 11:10:13

What I am doing in the IF statement is checking to see if num has been incremented during my select by a user, hence I assign @tempnum to num + 1 which is what num would now bw if it had been incremented by a user so in effect num could equal num + 1. It is jsut some validation I am doing.

Do you have any problem with the fact I am doing 3 select statements one after another??

nathans
Aged Yak Warrior

938 Posts

Posted - 2005-06-30 : 11:21:27

I dont think that is really validating anything.

You are checking to see if genNum.num was incremented between these two selects by another user??

SELECT @Num = Num FROM GenNum WHERE @serOff = SO 

SET @tempNum = @Num + 1

SELECT @tempNumTwo = Num FROM GenNum WHERE @serOff = SO

Is num an Identity column?

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2005-06-30 : 12:27:19

quote:
Originally posted by macca

What I am doing in the IF statement is checking to see if num has been incremented during my select by a user

You're checking if it was incremented once between those two SELECTs. What happens when it's incremented between the SELECTs and the UPDATE?
That is such a wrong-headed bit of code... but it is quite funny.

I suspect what you're trying to achieve is something like this:


CREATE PROCEDURE sproc_Number
(
@deptID int,
@serOff varchar(4) OUTPUT,
@returnNum int OUTPUT
)

AS

UPDATE G
SET @returnNum = G.Num = G.Num + 1, @serOff = S.serOff
FROM GenNum AS G
INNER JOIN (
    SELECT RTRIM(Service) + RTRIM(Office) AS serOff
    FROM Service
    WHERE DeptID = @deptID
  ) AS S
  ON G.SO = S.serOff

SELECT @returnNum
GO

macca
Posting Yak Master

146 Posts

Posted - 2005-07-01 : 04:29:36

Arnold,

Thanks for the reply. Glad you find my code humorous.

I am selecting num, but num could be selected by another user alo and incremented by that user. What I want to see is has num been incremented by another user since I selected it and if so I increment by 1.

Maybe I don't need to do this?
Is there another way to check if num has been incremented?

Can you answer any of my questions?
If not you can keep any wrong-headed comments.

Thanks,

macca

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2005-07-01 : 04:54:52

Ah, nice to get a response

What's wrong with the code I posted?

macca
Posting Yak Master

146 Posts

Posted - 2005-07-01 : 04:59:49

Arnold,

Can you explain your code and whatit does please.

Thanks

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2005-07-01 : 05:11:49

quote:
Originally posted by macca
Can you explain your code and whatit does please.

It increments GenNum.Num for the row(s) referring (via the Service table) to the deptID parameter passed in, and outputs the SO code and new Num value for one of those rows, as well as producing a rowset containing one row with the same Num value.
Is that not what you were trying to achieve?

macca
Posting Yak Master

146 Posts

Posted - 2005-07-01 : 05:34:40

Arnold, your code doesn't seem to work.
Are you sure it is written corerectly, I see you have Update...SET...FROM I never saw this type coding before.
Are you sure this is correct??

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2005-07-01 : 06:01:09

Ok, what assumptions am I getting wrong?

Here's what I tested it with: this works for me:
Edit: pardon me, I forgot the output parameters


CREATE TABLE Service (
  DeptID int PRIMARY KEY,
  Service char(2) NOT NULL,
  Office char(2) NOT NULL
)

CREATE TABLE GenNum (
  SO varchar(4) PRIMARY KEY,
  Num int NOT NULL
)

GO

INSERT INTO Service SELECT 3, 'AB', 'CD'
INSERT INTO Service SELECT 2, 'E', 'G'
INSERT INTO GenNum SELECT 'ABCD', 47
INSERT INTO GenNum SELECT 'EG', 2

GO

CREATE PROCEDURE sproc_Number
(
@deptID int,
@serOff varchar(4) OUTPUT,
@returnNum int OUTPUT
)

AS

UPDATE G
SET @returnNum = G.Num = G.Num + 1, @serOff = S.serOff
FROM GenNum AS G
INNER JOIN (
    SELECT RTRIM(Service) + RTRIM(Office) AS serOff
    FROM Service
    WHERE DeptID = @deptID
  ) AS S
  ON G.SO = S.serOff

SELECT @returnNum
GO

DECLARE @so varchar(4), @rn int

EXEC dbo.sproc_Number 3, @so OUTPUT, @rn OUTPUT
PRINT @so
PRINT @rn
EXEC dbo.sproc_Number 2, @so OUTPUT, @rn OUTPUT
PRINT @so
PRINT @rn
EXEC dbo.sproc_Number 3, @so OUTPUT, @rn OUTPUT
PRINT @so
PRINT @rn
EXEC dbo.sproc_Number 2, @so OUTPUT, @rn OUTPUT
PRINT @so
PRINT @rn

macca
Posting Yak Master

146 Posts

Posted - 2005-07-01 : 06:33:05

Thanks Arnold that code works. Thanks for all your help.
I have changed the sql bit, take a lokk at it and see what you think, I changed it because I need to output @Ser and @Off.

Here is the code:

CREATE PROCEDURE sproc_Number
(
@deptID int,
@Ser varchar(4) OUTPUT,
@Off varchar(4) OUTPUT,
@returnNum int OUTPUT
)

AS

DECLARE @serOff varchar(4)

UPDATE G
SET @returnNum = G.Num = G.Num + 1, @Ser = S.Service, @Off = S.Office, @serOff = S.serOff
FROM GenNum AS G
INNER JOIN (
SELECT RTRIM(Service) AS Service, RTRIM(Office) AS Office, RTRIM(Service) + RTRIM(Office) AS serOff
FROM Service
WHERE DeptID = @deptID
) AS S
ON G.SO = S.serOff

SELECT @returnNum
GO

Can I do any checks on the num as I said earlier or will I just use the sql as is??

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2005-07-01 : 06:49:06

The @serOff local variable isn't doing anything useful, but apart from that, yes it looks fine.

quote:

Can I do any checks on the num as I said earlier or will I just use the sql as is??

Using one UPDATE eliminates the possibility of there being a "between" during which other users can change the tables, so no, you shouldn't need to be doing the sort of checking you had in there originally.

macca
Posting Yak Master

146 Posts

Posted - 2005-07-01 : 09:07:20

Arnold thanks for the help.
I'll recommend you to others.
macca

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources