Skip to content

Commit 4337ee1

Browse files
author
MPCoreDeveloper
committed
fix(where): parenthesized OR/AND evaluate correctly + fail-closed unrecognized predicates (#348)
Root cause: a WHERE wrapped in redundant parentheses, e.g. (col = @p0 OR col = @p1), was passed to the single-condition evaluator as one condition, so the operator scan found '=' with a malformed column name '(col' -> no row matched (0 rows). The single-file OR/AND splitter and the directory token-walker (EvaluateJoinWhere) both ignored parentheses at depth > 0, and unrecognized conditions fell through to 'return true' (accept ALL rows). Fixes: - SqlInPredicate: add shared StripOuterParentheses + SplitTopLevelLogical helpers (parenthesis/string-literal aware), used by every WHERE evaluation path. - SingleFileTable.EvaluateCondition: strip redundant outer parens before OR/AND split; use the shared splitter (removed private duplicates); unrecognized single conditions now fail closed (return false) instead of accepting every row. - Table.EvaluateWhere (directory): strip outer parens and split AND/OR recursively (parenthesized sub-expressions such as 'a = 1 AND (b = 2 OR c = 3)' now work); BETWEEN is excluded from the split (contains 'AND'). - SqlParser.EvaluateJoinWhere: strip outer parens (defensive, covers PageBasedScan). - DatabaseExtensions.EvaluateSingleCondition: fail closed (dead path, same tautology). Verified: the reporter's probe expectations for multi-value IN/VALUES/OR were non-discriminating (both values matched all 3 seeded rows, so 3 rows is CORRECT). New discriminating regression tests (non-matching values) cover literal/param/VALUES lists, OR, parenthesized OR and AND+OR in single-file + directory + the EF Core provider path. 1,630 SharpCoreDB.Tests + 116 EF Core tests green.
1 parent 82f1e2d commit 4337ee1

7 files changed

Lines changed: 579 additions & 84 deletions

File tree

‎src/SharpCoreDB/DataStructures/Table.Scanning.cs‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,15 +206,34 @@ private bool EvaluateWhere(Dictionary<string, object> row, string? where)
206206
{
207207
if (string.IsNullOrEmpty(where)) return true;
208208

209+
// ✅ Issue #348: strip redundant outer parentheses so "(a = 1 OR b = 2)" and
210+
// "(a = 1 AND b = 2)" evaluate like their unparenthesized forms (the legacy
211+
// split-based evaluator treats a leading "(" as part of the column name).
212+
where = SqlInPredicate.StripOuterParentheses(where);
213+
209214
var parts = where.Split(' ', StringSplitOptions.RemoveEmptyEntries);
210215
if (parts.Length < 3) return true;
211216

212-
// ✅ FIX: Detect complex WHERE clauses (contains AND/OR) and delegate to SqlParser.EvaluateJoinWhere
213-
// which has full support for compound conditions
214-
var whereUpper = where.ToUpperInvariant();
215-
if (whereUpper.Contains(" AND ") || whereUpper.Contains(" OR "))
217+
// ✅ Issue #348: split compound conditions on top-level AND/OR and evaluate each
218+
// operand recursively (via the full single-condition evaluator below). The previous
219+
// delegation to SqlParser.EvaluateJoinWhere was a space-token walker that could not
220+
// handle parenthesized sub-expressions such as "a = 1 AND (b = 2 OR c = 3)".
221+
// BETWEEN contains "AND" as part of its syntax, so it is excluded from the split
222+
// (same guard as SingleFileTable.EvaluateCondition).
223+
bool hasBetween = where.Contains("BETWEEN", StringComparison.OrdinalIgnoreCase);
224+
if (!hasBetween)
216225
{
217-
return SqlParser.EvaluateJoinWhere(row, where);
226+
var orParts = SqlInPredicate.SplitTopLevelLogical(where, "OR");
227+
if (orParts.Count > 1)
228+
{
229+
return orParts.Any(part => EvaluateWhere(row, part));
230+
}
231+
232+
var andParts = SqlInPredicate.SplitTopLevelLogical(where, "AND");
233+
if (andParts.Count > 1)
234+
{
235+
return andParts.All(part => EvaluateWhere(row, part));
236+
}
218237
}
219238

220239
// ✅ Issue #339/#340: support IN / NOT IN lists for all column types, including

‎src/SharpCoreDB/DatabaseExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1473,7 +1473,7 @@ private bool EvaluateSingleCondition(Dictionary<string, object> row, string cond
14731473

14741474
if (op == null || opIndex < 0)
14751475
{
1476-
return true;
1476+
return false;
14771477
}
14781478

14791479
var columnName = condition.Substring(0, opIndex).Trim();

‎src/SharpCoreDB/Services/SqlInPredicate.cs‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,148 @@ public static bool IsMatch(Dictionary<string, object> row, ParsedInPredicate par
145145
public static bool IsMatch(object? rowValue, IEnumerable<string> items)
146146
=> items.Contains(rowValue?.ToString() ?? string.Empty);
147147

148+
/// <summary>
149+
/// Removes redundant outer parentheses from a logical condition so that
150+
/// <c>"(a = 1 OR b = 2)"</c> evaluates exactly like <c>"a = 1 OR b = 2"</c>.
151+
/// Only parentheses that enclose the WHOLE expression are stripped (e.g.
152+
/// <c>"(a = 1) OR (b = 2)"</c> is left intact), and parentheses inside string
153+
/// literals are ignored. Used by every WHERE evaluation path so parenthesized
154+
/// OR/AND predicates filter correctly (GitHub issue #348).
155+
/// </summary>
156+
public static string StripOuterParentheses(string condition)
157+
{
158+
var trimmed = condition.Trim();
159+
160+
while (trimmed.Length >= 2 && trimmed[0] == '(' && trimmed[^1] == ')')
161+
{
162+
int depth = 0;
163+
bool inString = false;
164+
char quote = '\0';
165+
bool fullyWrapped = true;
166+
167+
// Scan up to (but excluding) the final ')' — if the depth returns to 0 before
168+
// the end, the outer parens do not wrap the whole expression and must be kept.
169+
for (int i = 0; i < trimmed.Length - 1; i++)
170+
{
171+
char c = trimmed[i];
172+
173+
if (inString)
174+
{
175+
inString = c != quote; // closing quote exits the string literal
176+
continue;
177+
}
178+
179+
if (c is '\'' or '"')
180+
{
181+
inString = true;
182+
quote = c;
183+
continue;
184+
}
185+
186+
if (c == '(')
187+
{
188+
depth++;
189+
}
190+
else if (c == ')')
191+
{
192+
depth--;
193+
if (depth == 0)
194+
{
195+
fullyWrapped = false;
196+
break;
197+
}
198+
}
199+
}
200+
201+
// The scan excludes the final ')' (which balances the outer '('), so a fully
202+
// wrapped expression leaves exactly ONE unmatched '(' (depth == 1). If the depth
203+
// returns to 0 before the end, the outer parens do not wrap the whole expression
204+
// and must be kept.
205+
if (!fullyWrapped || depth != 1)
206+
{
207+
break;
208+
}
209+
210+
trimmed = trimmed[1..^1].Trim();
211+
}
212+
213+
return trimmed;
214+
}
215+
216+
/// <summary>
217+
/// Splits a condition on a logical keyword (AND / OR) that appears at the top level only —
218+
/// i.e. not inside parentheses or string literals. This keeps <c>IN ('a', 'b')</c> and
219+
/// <c>(a = 1 OR b = 2)</c> intact while still splitting <c>col = 1 OR col = 2</c>.
220+
/// </summary>
221+
public static List<string> SplitTopLevelLogical(string text, string keyword)
222+
{
223+
var parts = new List<string>();
224+
int depth = 0;
225+
bool inString = false;
226+
char quote = '\0';
227+
int start = 0;
228+
int i = 0;
229+
230+
while (i < text.Length)
231+
{
232+
char c = text[i];
233+
if (inString)
234+
{
235+
inString = c != quote; // closing quote exits the string literal
236+
i++;
237+
continue;
238+
}
239+
240+
if (c is '\'' or '"')
241+
{
242+
inString = true;
243+
quote = c;
244+
}
245+
else if (c == '(')
246+
{
247+
depth++;
248+
}
249+
else if (c == ')')
250+
{
251+
depth = Math.Max(0, depth - 1);
252+
}
253+
else if (IsLogicalKeywordAt(text, i, keyword, depth))
254+
{
255+
parts.Add(text[start..i].Trim());
256+
i += 1 + keyword.Length;
257+
while (i < text.Length && char.IsWhiteSpace(text[i]))
258+
{
259+
i++;
260+
}
261+
262+
start = i;
263+
continue;
264+
}
265+
266+
i++;
267+
}
268+
269+
parts.Add(text[start..].Trim());
270+
return parts;
271+
}
272+
273+
/// <summary>
274+
/// True when <paramref name="keyword"/> (OR / AND) starts right after a top-level space at
275+
/// <paramref name="index"/> and is followed by whitespace, e.g. <c>"col = 1 OR col = 2"</c>.
276+
/// </summary>
277+
private static bool IsLogicalKeywordAt(string text, int index, string keyword, int depth)
278+
{
279+
if (depth != 0 || text[index] is not (' ' or '\t'))
280+
{
281+
return false;
282+
}
283+
284+
var after = index + 1 + keyword.Length;
285+
return after < text.Length
286+
&& text.AsSpan(index + 1, keyword.Length).Equals(keyword.AsSpan(), StringComparison.OrdinalIgnoreCase)
287+
&& char.IsWhiteSpace(text[after]);
288+
}
289+
148290
/// <summary>
149291
/// Evaluates a raw IN value list (e.g. <c>('a', 'b')</c>, <c>(1,2,3)</c> or
150292
/// <c>(VALUES ('a'), ('b'))</c>) against a single row value. Commas inside parentheses

‎src/SharpCoreDB/Services/SqlParser.Helpers.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,10 @@ public static bool EvaluateJoinWhere(Dictionary<string, object> row, string wher
300300
return true;
301301
}
302302

303+
// ✅ Issue #348: strip redundant outer parentheses so "(a = 1 OR b = 2)" evaluates
304+
// like "a = 1 OR b = 2" (a leading "(" would otherwise stick to the column name).
305+
where = SqlInPredicate.StripOuterParentheses(where);
306+
303307
var parts = where.Split(' ');
304308
if (parts.Length <= 3)
305309
{

‎src/SharpCoreDB/SingleFileTable.cs‎

Lines changed: 16 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,6 +1125,10 @@ private static string NormalizeColumnName(string columnName)
11251125
{
11261126
var trimmed = condition.Trim();
11271127

1128+
// ✅ Issue #348: strip redundant outer parentheses so "(a = 1)" is parsed with the
1129+
// bare column name ("a") instead of a malformed one ("(a").
1130+
trimmed = SqlInPredicate.StripOuterParentheses(trimmed);
1131+
11281132
// Conservative eligibility: reject any condition that the full evaluator handles
11291133
// with dedicated syntax (AND/OR chains, IN lists, LIKE, BETWEEN, IS [NOT] NULL).
11301134
// Rejecting is always safe — the fallback preserves existing behavior.
@@ -1253,21 +1257,26 @@ private static bool EvaluateCondition(Dictionary<string, object> row, string con
12531257
{
12541258
var trimmedCondition = condition.Trim();
12551259

1260+
// ✅ Issue #348: strip redundant outer parentheses so "(a = 1 OR b = 2)" is split on
1261+
// OR like the unparenthesized form instead of being treated as one condition with a
1262+
// malformed column name ("(a").
1263+
trimmedCondition = SqlInPredicate.StripOuterParentheses(trimmedCondition);
1264+
12561265
// ✅ Parity: BETWEEN contains " AND " as part of its syntax; don't split on it.
12571266
if (trimmedCondition.Contains("BETWEEN", StringComparison.OrdinalIgnoreCase))
12581267
{
12591268
return EvaluateSingleCondition(row, trimmedCondition);
12601269
}
12611270

1262-
// ✅ Issue #340: handle OR chains (e.g. col = @p0 OR col = @p1). Split on top-level
1271+
// ✅ Issue #348: handle OR chains (e.g. col = @p0 OR col = @p1). Split on top-level
12631272
// OR first — any matching branch makes the whole condition true.
1264-
var orParts = SplitTopLevelLogical(trimmedCondition, "OR");
1273+
var orParts = SqlInPredicate.SplitTopLevelLogical(trimmedCondition, "OR");
12651274
if (orParts.Count > 1)
12661275
{
12671276
return orParts.Any(part => EvaluateCondition(row, part));
12681277
}
12691278

1270-
var parts = SplitTopLevelLogical(trimmedCondition, "AND");
1279+
var parts = SqlInPredicate.SplitTopLevelLogical(trimmedCondition, "AND");
12711280
if (parts.Count > 1)
12721281
{
12731282
return parts.All(part => EvaluateCondition(row, part));
@@ -1276,80 +1285,6 @@ private static bool EvaluateCondition(Dictionary<string, object> row, string con
12761285
return EvaluateSingleCondition(row, trimmedCondition);
12771286
}
12781287

1279-
/// <summary>
1280-
/// Splits a condition on a logical keyword (AND / OR) that appears at the top level only —
1281-
/// i.e. not inside parentheses or string literals. This keeps <c>IN ('a', 'b')</c> and
1282-
/// <c>(a = 1 OR b = 2)</c> intact while still splitting <c>col = 1 OR col = 2</c>.
1283-
/// </summary>
1284-
private static List<string> SplitTopLevelLogical(string text, string keyword)
1285-
{
1286-
var parts = new List<string>();
1287-
int depth = 0;
1288-
bool inString = false;
1289-
char quote = '\0';
1290-
int start = 0;
1291-
int i = 0;
1292-
1293-
while (i < text.Length)
1294-
{
1295-
char c = text[i];
1296-
if (inString)
1297-
{
1298-
inString = c != quote; // closing quote exits the string literal
1299-
i++;
1300-
continue;
1301-
}
1302-
1303-
if (c is '\'' or '"')
1304-
{
1305-
inString = true;
1306-
quote = c;
1307-
}
1308-
else if (c == '(')
1309-
{
1310-
depth++;
1311-
}
1312-
else if (c == ')')
1313-
{
1314-
depth = Math.Max(0, depth - 1);
1315-
}
1316-
else if (IsLogicalKeywordAt(text, i, keyword, depth))
1317-
{
1318-
parts.Add(text[start..i].Trim());
1319-
i += 1 + keyword.Length;
1320-
while (i < text.Length && char.IsWhiteSpace(text[i]))
1321-
{
1322-
i++;
1323-
}
1324-
1325-
start = i;
1326-
continue;
1327-
}
1328-
1329-
i++;
1330-
}
1331-
1332-
parts.Add(text[start..].Trim());
1333-
return parts;
1334-
}
1335-
1336-
/// <summary>
1337-
/// True when <paramref name="keyword"/> (OR / AND) starts right after a top-level space at
1338-
/// <paramref name="index"/> and is followed by whitespace, e.g. <c>"col = 1 OR col = 2"</c>.
1339-
/// </summary>
1340-
private static bool IsLogicalKeywordAt(string text, int index, string keyword, int depth)
1341-
{
1342-
if (depth != 0 || text[index] is not (' ' or '\t'))
1343-
{
1344-
return false;
1345-
}
1346-
1347-
var after = index + 1 + keyword.Length;
1348-
return after < text.Length
1349-
&& text.AsSpan(index + 1, keyword.Length).Equals(keyword.AsSpan(), StringComparison.OrdinalIgnoreCase)
1350-
&& char.IsWhiteSpace(text[after]);
1351-
}
1352-
13531288
private static bool EvaluateSingleCondition(Dictionary<string, object> row, string condition)
13541289
{
13551290
var trimmed = condition.Trim();
@@ -1433,7 +1368,10 @@ private static bool EvaluateSingleCondition(Dictionary<string, object> row, stri
14331368

14341369
if (op == null || opIndex < 0)
14351370
{
1436-
return true;
1371+
// ✅ Issue #348: fail closed — an unrecognized condition must NOT accept every
1372+
// row (the old "return true" turned malformed/unsupported predicates into a
1373+
// tautology that silently returned the whole table).
1374+
return false;
14371375
}
14381376

14391377
var columnName = condition[..opIndex].Trim();

0 commit comments

Comments
 (0)