December 5, 2016 at 7:14 pm
Trying to demonstrate that existing code is deleting more than it should.
But I can't figure out how to complete this test.
DECLARE @Results TABLE
(
Result FLOAT,
Result_on DATETIME,
Target FLOAT
)
DECLARE @ResultsShadow TABLE -- my addition for testing
(
result FLOAT,
result_on DATETIME,
Target FLOAT
)
DECLARE @Alarms TABLE
(
Id INT IDENTITY,
StartTime DATETIME,
EndTime DATETIME,
Action1 INT
)
DECLARE @DebugTable TABLE -- my addition for testing
(
RowsAffected INT, -- ignore
SanityCheck INT, -- ignore
AlarmStart DATETIME,
AlarmEnd DATETIME,
Result_On DATETIME,
AlarmId INT,
[Target] REAL,
Result REAL
)
SET @i = 1
WHILE @i IS NOT NULL
BEGIN
SELECT @AlarmStart = StartTime, @AlarmEnd = EndTime
FROM @Alarms
WHERE Id = @i
IF (@IncludeAlarms = 0)
BEGIN
DELETE FROM @Results WHERE Result_On >= @AlarmStart -- original condition
AND Result_On < @AlarmEnd -- original condition
OUTPUT DELETED.* INTO @ResultsShadow-- from here down to...
BEGIN
INSERT INTO @DebugTable(
Result_On,
AlarmId,
[Target],
Result)
SELECT
@AlarmStart,
@AlarmEnd,
Result_On
FROM @Results
END
END -- ... here is my feeble additions
SELECT @i = min(Id) FROM @Alarms WHERE id > @i
END
The while loop is iterating through the @Alarms table, and selecting a value for @AlarmStart and @AlarmEnd.
Then (based on a condition) each row of @Results table is compared with these values and any matches found are deleted.
I know more than one row is deleted from @Results during each iteration because I used @@ROWCOUNT to prove it.
The problem is there should be only a one-to-one match. At most only one row should be deleted per iteration/match. And I need to capture more information about those deletions to highlight the problem.
But I'm having a hard time figuring out how to grab that info and putting it, along with the @AlarmStart, @AlarmEnd condition, into the @DebugTable, from within the loop.
I have to keep this loop structure intact because this is existing code and easier to demonstrate the problem if I'm not re-writing it or making significant changes.
So I'm just trying to capture the details of what it is deleting and the conditions for each deletion.
I thought maybe if I OUTPUTTED the deleted rows into the @ResultsShadow table (that I made), then I could use
it's contents and push them into the @DebugTable, along with the respective @AlarmStart, @AlarmEnd values.
Then clear the @ResultsShadow table for the next iteration.
But I couldn't get that to work and maybe there is a better way.
Suggestions?
UPDATE - thanks to John for pointing out that the WHERE goes after the OUTPUT.
SET @i = 1
WHILE @i IS NOT NULL
BEGIN
SELECT @AlarmStart = StartTime, @AlarmEnd = EndTime,
FROM @Alarms
WHERE Id = @i
IF (@IncludeAlarms = 0)
BEGIN
DELETE FROM @Results
OUTPUT DELETED.* INTO @ResultsShadow
WHERE Result_On >= @AlarmStart
AND Result_On < @AlarmEnd
IF @@ROWCOUNT > 0
BEGIN
INSERT INTO @DebugTable(
AlarmStart,
AlarmEnd,
Result_On,
AlarmId,
[Target],
Result)
SELECT
@AlarmStart, -- from this iteration, just one val
@AlarmEnd, -- from this iteration, just one val
result_on, -- from @ResultsShadow
@i, -- from this iteration, just one val
[Target], -- from @ResultsShadow
result -- from @ResultsShadow
FROM @ResultsShadow
DELETE FROM @ResultsShadow-- empty it for next iteration
END
END
SELECT @i = min(Id) FROM @Alarms WHERE id > @i
END
December 6, 2016 at 1:26 am
Quick question, have you tested if @i ever becomes NULL?
😎
WHILE @i IS NOT NULL
...
SELECT @i = min(Id) FROM @Alarms WHERE id > @i
December 6, 2016 at 4:17 am
Eirikur Eiriksson (12/6/2016)
Quick question, have you tested if @i ever becomes NULL?😎
WHILE @i IS NOT NULL...
SELECT @i = min(Id) FROM @Alarms WHERE id > @i
Uh... no. I assume the while loop would then exit if @i becomes NULL. I'm not following your train of thought.
December 6, 2016 at 4:48 am
Not sure whether this is an option for you, but my suggestion is just to ditch the loop. You can replace the whole lot with something like this:DELETE FROM @Results
WHERE Result_On >= (SELECT MIN(StartTime) FROM @Alarms)
AND Result_On < (SELECT MAX(EndTime) FROM @Alarms)
You could even rewrite the above so it only needs to do a single scan of the @Alarms table, something like this:DELETE r
FROM @Results r
JOIN (
SELECT
MIN(StartTime) AS EarliestStart
,MAX(EndTime) AS LatestEnd
) m
ON r.Result_On >= m.EarliestStart
AND r.Result_On < m.LatestEnd
With no sample data, I haven't tested either of these two queries, but even if they don't work first time, they'll be a place to start.
John
December 6, 2016 at 4:56 am
John Mitchell-245523 (12/6/2016)
Not sure whether this is an option for you, but my suggestion is just to ditch the loop. You can replace the whole lot with something like this:DELETE FROM @ResultsWHERE Result_On >= (SELECT MIN(StartTime) FROM @Alarms)
AND Result_On < (SELECT MAX(EndTime) FROM @Alarms)
You could even rewrite the above so it only needs to do a single scan of the @Alarms table, something like this:
DELETE rFROM @Results r
JOIN (
SELECT
MIN(StartTime) AS EarliestStart
,MAX(EndTime) AS LatestEnd
) m
ON r.Result_On >= m.EarliestStart
AND r.Result_On < m.LatestEnd
With no sample data, I haven't tested either of these two queries, but even if they don't work first time, they'll be a place to start.
John
Unfortunately it is not an option. Yeah even my noobie-SQL senses asked if that loop is even necessary.
So I'm not surprised that there are better alternatives.
But I can't change the code without completely justifying it.
Which brings me back to my main point of trying to demonstrate the faulty logic of this code - if there is any - by gathering more information regarding the DELETE section.
December 6, 2016 at 5:05 am
OK, fair enough.
The OUTPUT clause goes before the WHERE clause. Are you not getting a syntax error?
Some sample data would be very helpful here. If the StartTime to EndTime period on any of your rows overlaps with that of any other row, then you can't guarantee a one-to-one mapping.
John
December 6, 2016 at 8:39 am
Just include the @id and id values in your debug table.
INSERT INTO @DebugTable(
input_id,
id,
Result_On,
AlarmId,
[Target],
Result)
SELECT
@id,
id,
@AlarmStart,
@AlarmEnd,
Result_On
FROM @Results
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
Viewing 7 posts - 1 through 6 (of 6 total)
You must be logged in to reply to this topic. Login to reply