Skip to content

DBE-3763 Fix for known issues#47

Open
ramyamasani wants to merge 1 commit into
masterfrom
dbe3763_fix_known_issues
Open

DBE-3763 Fix for known issues#47
ramyamasani wants to merge 1 commit into
masterfrom
dbe3763_fix_known_issues

Conversation

@ramyamasani
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@orca-security-us orca-security-us Bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds JSON support for MySQL and updates the _jsons_equiv utility to treat database NULL values as JSON null literals, ensuring consistent comparisons across different database backends. Feedback suggests optimizing the comparison logic by adding an early equality check after normalization to avoid unnecessary JSON parsing and updating the function's type hints to reflect that it now accepts None values.

Comment thread data_diff/utils.py
Comment on lines +519 to +522
if a is None:
a = "null"
if b is None:
b = "null"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

After normalizing None values to the string "null", it is more efficient to check for string equality before proceeding to parse the JSON. This avoids the overhead of two json.loads() calls when comparing a database NULL (which becomes "null") with a JSON "null" literal, which is the primary use case for this change. Additionally, since this function now explicitly handles None values, the type hints in the function signature should ideally be updated to Optional[str] to reflect this.

    if a is None:
        a = "null"
    if b is None:
        b = "null"
    if a == b:
        return True

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.

1 participant