Fix bug where some oracles were not reporting their findings - #1364
Merged
Merged
Conversation
…se-reducer was enabled
…d as diff rows in logs
generateUnoptimizedQueryString projected COUNT(*) and cleared the WHERE clause without re-projecting the predicate, so the unoptimized side counted every row of the table regardless of the condition. Every run whose predicate filtered anything therefore reported a mismatch. Project the predicate per row instead, as the other NoREC generators do. A CASE expression is used rather than a cast because HSQLDB does not allow casting a BOOLEAN to an integer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1XqNaswnH8mWTXFkkGa1X
generateOptimizedQueryString projected a literal COUNT(*) column in its non-aggregate branch as well as in its aggregate one. NoRECOracle counts the rows that branch returns, and an aggregate always returns exactly one, so the optimized count was 1 whatever the predicate matched. Project * in the non-aggregate branch, as PostgresExpressionGenerator does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1XqNaswnH8mWTXFkkGa1X
generateOptimizedQueryString set the WHERE clause only in its non-aggregate branch, so the aggregate branch counted every row of the table while the unoptimized side applied the predicate. Set the WHERE clause for both branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1XqNaswnH8mWTXFkkGa1X
The expression generator can produce chr() with a negative argument, on which YSQL raises "character number must be positive". That error was missing from YSQLErrors, so the oracle reported it as an unexpected error rather than ignoring the statement. PostgresCommon already lists it (PostgresCommon.java:87); YSQLErrors otherwise mirrors that list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1XqNaswnH8mWTXFkkGa1X
mrigger
approved these changes
Sep 14, 2026
mrigger
left a comment
mrigger
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Sounds like quite a serious bug. Great that you have found and fixed it!
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
2 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.
This PR solves a bug where any oracle supplying a reproducer would only report its findings when --use-reducer was enabled. This bug has been present since 2022 (commit 625204e), and I noticed it as I have been extending reproducer support to more oracles.
This meant the CI was not running correctly for some oracles (because the CI runs without --use-reducer). By solving this, the CI now runs correctly, which has surfaced some previously hidden defects. With the help of Claude, I then went on to fix those newly surfaced bugs for the affected DBMSs so the CI passes again:
HSQLDBExpressionGenerator.generateUnoptimizedQueryStringcleared the WHERE clause and projectedCOUNT(*)without re-projecting the predicate, making the unoptimised side count every row of the table. It now projects the predicate per row.CockroachDBExpressionGenerator.generateOptimizedQueryStringprojected aCOUNT(*)column without using a GROUP BY, which returns a single row with the count.countRowswas then called on this, which counts the number of rows, and therefore always counted one row instead of the actual underlying rows.CockroachDBExpressionGenerator.generateOptimizedQueryStringnow projects*instead ofCOUNT(*), socountRowscounts the actual rows.YSQLExpressionGenerator.generateOptimizedQueryStringset the WHERE clause only in its non-aggregate branch, so the aggregate branch counted every row while the unoptimized side applied the predicate; the WHERE clause is now set for both. Finally, YSQLErrors was missing "character number must be positive" as an expected error (which is already listed for Postgres).