Skip to content

Filter with unparenthesized AND/OR silently returns 0 rows instead of an error #172

Description

@nicdavidson

Summary

DbRequestCriteria::parseFilterString() only recognizes logical operators in the (a) AND (b) / (a)AND(b) form. A filter written as a='x' OR b='y' (no parentheses) is not rejected: it falls through to the single-condition path, the text after the first comparison operator is bound as the literal value, and the query quietly returns zero rows.

The docs do say to wrap conditions in parentheses, so this is arguably "documented", but silently matching nothing is the worst possible failure mode: callers get an empty result, not a 400, and assume the data isn't there. It bites hardest with AI clients: the MCP get_table_data / aggregate_data tools expose filter as a free-form string, and every LLM writes plain SQL-style AND/OR first.

Reproduce

Same behaviour on SQL Server (dev, dev-develop e9f13bb) and MySQL (stock df-docker 7.7.0, df-core 1.0.17):

filter rows expected
status='open' 24 24
status='open' OR status='closed' 0 32
(status='open') OR (status='closed') 32 32
status='open' AND district=3 0 3
(status='open') AND (district=3) 3 3
status in ('open','closed') 32 32

MySQL: name='Alder Peak Outfitters' OR name='zzz' → 0 rows; parenthesized → 1 row. Even name='x' AND name='x' → 0 rows.

Where

src/Components/DbRequestCriteria.php ~L217-252: the logical-operator loop only matches ') OP (' after normalising ')OP('; anything else is treated as one comparison and split on the first operator.

Suggested fix

Either of:

  1. Parse it. When no ) OP ( match is found, split on \s+(AND|OR)\s+ outside quotes and treat each piece as a condition (equivalent to auto-wrapping in parentheses). Precedence would be left-to-right, same as today's parenthesized form.
  2. Reject it. If the remaining single "condition" still contains a top-level AND/OR token outside quotes, throw BadRequestException("Logical operators must be parenthesized: (a) AND (b)") instead of binding it as a value.

Option 1 is friendlier to MCP/AI clients; option 2 is the minimum to stop silent empty results. Also worth updating the filter description in df-mcp-server's tool schema either way.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions