Conversation
…ding it _ancestors_noparent_condition() builds its WHERE clause by joining one term per prefix length above the block. At prefix 0 there is nothing above, so the join produced an empty string, which it then wrapped in parentheses and handed to the database as "()" -- a 1064 syntax error. That was worked around by giving each caller that could reach prefix 0 its own `if ip.prefix != 0` guard: in Ipblock._tree_update(), in _find_ipblock() and on the guess_function of ipblock_create(). The broken query stayed, waiting for the next caller. Fix the cause: an empty disjunction is false, so return false() and let callers get the empty result set that is the correct answer. The three guards then say nothing the query does not already say, and are removed. Verified behaviour-neutral: of the seven new tests, five pass both with and without this change -- they pin down exactly what the guards used to provide. The other two go at the query directly and are the ones that fail without it. Not addressed, found while reading: _find_ipblock() computes `status_str = ' or '.join(status)` unconditionally, but its `status` parameter defaults to None and ipblock_list() passes it through. Reachable over the API with a block that does not exist exactly; not reachable through ndcli, which never calls ipblock_list.
eschweikert
requested changes
Aug 21, 2026
eschweikert
left a comment
eschweikert
left a comment
Collaborator
There was a problem hiding this comment.
Cood looks good. Please add the sql sheme update
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ding it
_ancestors_noparent_condition() builds its WHERE clause by joining one term per prefix length above the block. At prefix 0 there is nothing above, so the join produced an empty string, which it then wrapped in parentheses and handed to the database as "()" -- a 1064 syntax error.
That was worked around by giving each caller that could reach prefix 0 its own
if ip.prefix != 0guard: in Ipblock._tree_update(), in _find_ipblock() and on the guess_function of ipblock_create(). The broken query stayed, waiting for the next caller.Fix the cause: an empty disjunction is false, so return false() and let callers get the empty result set that is the correct answer. The three guards then say nothing the query does not already say, and are removed.
Verified behaviour-neutral: of the seven new tests, five pass both with and without this change -- they pin down exactly what the guards used to provide. The other two go at the query directly and are the ones that fail without it.
Not addressed, found while reading: _find_ipblock() computes
status_str = ' or '.join(status)unconditionally, but itsstatusparameter defaults to None and ipblock_list() passes it through. Reachable over the API with a block that does not exist exactly; not reachable through ndcli, which never calls ipblock_list.Internal ticket: ITOUDP-5168