Skip to content

fix #33 #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025
Merged

fix #33 #43

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 35 additions & 6 deletions Direct_Framework/Stored Procedures/omd.AddBatchToParentBatch.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,46 @@ BEGIN TRY

-- Validate the DAG
BEGIN TRY
/*
TODO: Validate DAG, no circular references allowed...
*/

-- DAG Check: Ensure that adding @ParentBatchId -> @BatchId does not form a cycle
IF @CheckDag = 'Y'
BEGIN
PRINT('There should be some clever SQL added here to warn of circular relations...')
;WITH Ancestors AS (
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to include a comment explaining the recursive CTE's logic and how it retrieves the chain of ancestors for cycle detection.

Copilot uses AI. Check for mistakes.

SELECT BATCH_ID, PARENT_BATCH_ID
FROM [omd].[BATCH_HIERARCHY]
WHERE BATCH_ID = @ParentBatchId

UNION ALL

SELECT h.BATCH_ID, h.PARENT_BATCH_ID
FROM [omd].[BATCH_HIERARCHY] h
INNER JOIN Ancestors a ON h.BATCH_ID = a.PARENT_BATCH_ID
)
SELECT BATCH_ID
INTO #DagViolation
FROM Ancestors
WHERE PARENT_BATCH_ID = @BatchId;

IF EXISTS (SELECT 1 FROM #DagViolation)
BEGIN
SET @LogMessage = 'Circular relationship detected: adding BatchId ' + CONVERT(NVARCHAR(10), @BatchId) +
' under ParentBatchId ' + CONVERT(NVARCHAR(10), @ParentBatchId) +
' would create a cycle.';
SET @MessageLog = [omd].[AddLogMessage]('ERROR', DEFAULT, DEFAULT, @LogMessage, @MessageLog);
DROP TABLE IF EXISTS #DagViolation;
GOTO EndOfProcedureFailure;
Comment on lines +162 to +163
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GOTO for error handling can reduce the clarity and maintainability of the procedure. Consider refactoring the error handling to rely solely on the TRY/CATCH structure instead of branching with GOTO.

Suggested change
DROP TABLE IF EXISTS #DagViolation;
GOTO EndOfProcedureFailure;
SET @SuccessIndicator = 'N';
DROP TABLE IF EXISTS #DagViolation;
THROW 50001, @LogMessage, 1;

Copilot uses AI. Check for mistakes.

END

DROP TABLE IF EXISTS #DagViolation;
END


END TRY
BEGIN CATCH
-- TODO
SET @LogMessage = 'Error testing for circular relationships in the DAG check.';
SET @MessageLog = [omd].[AddLogMessage]('ERROR', DEFAULT, DEFAULT, @LogMessage, @MessageLog);
SET @SuccessIndicator = 'N';

THROW 50000, @LogMessage, 1
END CATCH

-- Find the Parent Batch Id
Expand Down