Skip to content

fix(sql): JIT compilation on symbol column equality filters leads to incorrect query result#6321

Merged
bluestreak01 merged 2 commits intomasterfrom
puzpuzpuz_disable_jit_diff_sym_eq_filters
Oct 28, 2025
Merged

fix(sql): JIT compilation on symbol column equality filters leads to incorrect query result#6321
bluestreak01 merged 2 commits intomasterfrom
puzpuzpuz_disable_jit_diff_sym_eq_filters

Conversation

@puzpuzpuz
Copy link
Copy Markdown
Contributor

@puzpuzpuz puzpuzpuz commented Oct 28, 2025

WHERE clause filters like sym_col1 = sym_col2 were mistakenly JIT compiled. The compiled filter function was using internal symbol codes for the equality check which produced incorrect results returned from the query. This patch fixes the problem by disabling JIT compilation for such filters.

@puzpuzpuz puzpuzpuz self-assigned this Oct 28, 2025
@puzpuzpuz puzpuzpuz added Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution labels Oct 28, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 28, 2025 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes modify the IR serialization layer to use a unified tree traversal algorithm and introduce runtime validation to prevent mixing different symbol columns in operator comparisons. Three new test cases verify this validation behavior throws exceptions appropriately.

Changes

Cohort / File(s) Summary
Production code - IR serialization updates
core/src/main/java/io/questdb/jit/CompiledFilterIRSerializer.java		 Replaced inPredicateTraverseAlgo with unified traverseAlgo instance (PostOrderTreeTraversalAlgo) across IN-list and timestamp-range IN handling paths. Added guard in PredicateContext.handleColumn to reject operations mixing different symbol columns via SqlException.
New test for symbol equality
core/src/test/java/io/questdb/test/griffin/engine/functions/eq/EqSymFunctionFactoryTest.java		 New test class extending AbstractCairoTest with testSmoke method verifying equality comparisons on randomly-generated symbol columns against expected output.
Symbol column validation tests
core/src/test/java/io/questdb/test/jit/CompiledFilterIRSerializerTest.java		 Added three new test cases verifying that comparing two SYMBOL columns using >, =, and != operators throws SqlException, validating the new cross-symbol-column mixing guard.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention areas: Verify that PostOrderTreeTraversalAlgo routing through the new traverseAlgo field maintains semantic equivalence with the previous inPredicateTraverseAlgo usage, particularly for IN-list and timestamp-range query paths.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "fix(sql): JIT compilation on symbol column equality filters leads to incorrect query result" is directly and specifically related to the changeset. The title clearly identifies the main change: fixing a bug where JIT compilation of symbol column equality filters (comparing two symbol columns) was producing incorrect query results. According to the raw summary, the changes involve introducing validation to reject mixing symbol columns and adding tests for this scenario, which aligns precisely with what the title describes. The title is concise, specific, and clearly conveys the primary problem being addressed without vague terminology.
Description Check ✅ Passed The PR description is closely related to the changeset and provides meaningful context about the fix. It explains the specific problem (WHERE clause filters comparing two symbol columns were being JIT compiled), the root cause (using internal symbol codes for equality checks), and the intended solution (disabling JIT compilation for such filters). This aligns with the changes visible in the raw summary, which include validation logic in CompiledFilterIRSerializer and new test cases verifying that cross-symbol-column comparisons throw SqlException. The description is neither vague nor generic, as it provides concrete details about the issue and the fix approach.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@puzpuzpuz
Copy link
Copy Markdown
Contributor Author

puzpuzpuz commented Oct 28, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 28, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@puzpuzpuz
Copy link
Copy Markdown
Contributor Author

puzpuzpuz commented Oct 28, 2025

@RaphDal thanks for the review!

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Oct 28, 2025

[PR Coverage check]

😍 pass : 12 / 12 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/jit/CompiledFilterIRSerializer.java 12 12 100.00%

@bluestreak01 bluestreak01 merged commit 4ee11ab into master Oct 28, 2025
34 checks passed
@bluestreak01 bluestreak01 deleted the puzpuzpuz_disable_jit_diff_sym_eq_filters branch October 28, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RaphDal RaphDal RaphDal approved these changes

Assignees

@puzpuzpuz puzpuzpuz

Labels

Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@puzpuzpuz @glasstiger @RaphDal @bluestreak01