Bugfix: NoOverlap2d constraint fails to recognize duplicates

[kklein]

Bug detected by @EgeKaraismailogluQC.

Failing example: 79aff3b

Fundamentally, the overlap detection contained a flaw in which the 'left' bound of two intervals could not be equal for them to be considered overlapping. A special case of this situation is when two intervals are exactly equal - the counterexample which made this bug apparent.

Importantly, this bug affects all overlap-related constraints.

The bug is fixed in datajudge.db_access.get_interval_overlaps_nd by:

• Replacing a strict inequality by an equality
• Explicitly handling the case where an n-dimensional interval is duplicated; if we didn't handle this explicitly we would always find an 'overlap' for a table is joined with itself

Test cases are added for all relevant overlap-constraint-adding methods.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.38%. Comparing base (efb059f) to head (58dfa87).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #256      +/-   ##
==========================================
+ Coverage   93.36%   93.38%   +0.02%     
==========================================
  Files          18       18              
  Lines        1988     1996       +8     
==========================================
+ Hits         1856     1864       +8     
  Misses        132      132              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kklein]

@ivergara I believe it is necessary to manually tear down some integration test data, since the content of some tables has changed and we don't have an automatic teardown for snowflake. Would you be so kind to do this @ivergara ? I don't think I have access to (and if so I have forgotten how to) enter the snowflake administration interface.

[EgeKaraismailogluQC]

@kklein thanks for tagging me here! I am not familiar enough with the codebase to approve or request changes, but I do have one question: why do we need to handle an exact match separately? It seems to me that our definition of an overlap in naive_violation_condition should cover exact matches.

[kklein]

Thanks for taking a look @EgeKaraismailogluQC .

why do we need to handle an exact match separately?

In order to determine whether there are overlaps within a table, we merge the table onto itself with the 'overlap condition' as a join condition. Hence naive_violation_condition will always find a violation: a row overlaps with itself.

Does that make sense?

[ivergara]

@ivergara I believe it is necessary to manually tear down some integration test data, since the content of some tables has changed and we don't have an automatic teardown for snowflake. Would you be so kind to do this @ivergara ? I don't think I have access to (and if so I have forgotten how to) enter the snowflake administration interface.

Will take a look. I do still know how to log in, and I hope that by deleting the data it'll be enough.

[EgeKaraismailogluQC]

@kklein makes sense, thanks!

[kklein]

Thanks @ivergara - the tests are passing now :)
Do you think you could review the PR?