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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sjohansson
Copy link
Contributor

add initial version of circular relationship testing for testing

add initial version of circular relationship testing for testing
@sjohansson sjohansson requested review from RoelantVos and Copilot May 20, 2025 11:30
@sjohansson sjohansson linked an issue May 20, 2025 that may be closed by this pull request
1 task
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements an initial version of circular relationship detection for batch hierarchies in the stored procedure. It adds a recursive CTE to traverse the DAG, performs cycle detection by verifying that the new relationship does not introduce a cycle, and logs an error when a cycle is found.

Comment on lines +162 to +163
DROP TABLE IF EXISTS #DagViolation;
GOTO EndOfProcedureFailure;
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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add batch hierarchy circular reference checks
1 participant