Skip to content

fix tag file crash - #34607

Merged
guanshengliang merged 2 commits into
mainfrom
fix/fixTagFilterCrash
Mar 9, 2026
Merged

guanshengliang merged 2 commits into
mainfrom
fix/fixTagFilterCrash

Conversation

@yihaoDeng

Copy link
Copy Markdown
Contributor

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

Copilot AI review requested due to automatic review settings February 27, 2026 08:16
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a severe crash encountered during queries that compare two tag columns. The solution involves refining the index filtering mechanism to correctly identify and bypass index usage for column-to-column comparisons, thereby ensuring query stability and proper execution. A dedicated test case has been added to confirm the robustness of this fix across different comparison types.

Highlights

  • Crash Fix for Tag Comparisons: Addressed a critical crash that occurred when executing queries comparing two tag columns (e.g., tag1=tag2). The fix prevents the index filter from being used in such scenarios, allowing the query to fall back to a non-indexed execution path.
  • Index Filter Logic Enhancement: Improved the sifInitOperParams function to validate that the right operand of a binary operation is a value node when expected, and enhanced sifShouldUseIndexBasedOnType to explicitly disable index filtering for column-to-column comparisons.
  • New Test Case for Tag-to-Tag Comparisons: Introduced a new test method, do_tag_column_comparison, in test_index_tag_basic.py to thoroughly verify that tag-to-tag comparisons work correctly without crashing for various operators (=, <, >).
  • CI Integration: Integrated the test_index_tag_basic.py test suite into the CI pipeline to ensure continuous validation of tag index functionalities.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • source/libs/index/src/indexFilter.c
    • Added a check to sifInitOperParams to ensure the right operand is a value node for specific operations, preventing invalid input.
    • Introduced logic in sifShouldUseIndexBasedOnType to disable index filtering for column-to-column comparisons, preventing crashes.
  • test/cases/15-TagIndices/test_index_tag_basic.py
    • Added a new test method do_tag_column_comparison to validate tag-to-tag comparisons.
    • Updated the main test function to include the execution of the new tag-to-tag comparison test.
    • Commented out a prepareData() call in do_ts4403.
  • test/ci/cases.task
    • Included test_index_tag_basic.py in the CI test execution list.
Activity
  • The pull request description indicates a fix for a tag file crash and includes a standard checklist for manual updates, test cases, and test coverage, none of which are marked as completed in the provided context.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to fix a crash that occurs when performing a column-to-column comparison on tags, such as WHERE tag1 = tag2. The fix involves adding a check to bypass the index filter for such comparisons, allowing the query to execute through a fallback path. A new test case is also added to verify this fix and prevent future regressions.

The core logic to detect and bypass the index for column-to-column comparisons in sifShouldUseIndexBasedOnType is correct. However, another change in sifInitOperParams seems to incorrectly introduce a hard failure for these same queries, which contradicts the goal of the PR. I've left a specific comment on that part of the code.

Comment thread source/libs/index/src/indexFilter.c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request addresses a crash in the tag index filter when executing column-to-column comparison queries (e.g., SELECT * FROM st WHERE tag1=tag2). The crash occurred because the index filter code attempted to dereference a NULL condValue pointer when the right operand was a column reference rather than a literal value.

Changes:

  • Added a NULL check in sifShouldUseIndexBasedOnType to prevent using index filters for column-to-column comparisons
  • Added early validation in sifInitOperParams to reject non-VALUE right operands
  • Added comprehensive test case do_tag_column_comparison to verify the fix
  • Commented out the existing do_ts4403 test that appears to have issues

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
source/libs/index/src/indexFilter.c Added validation and NULL checks to handle column-to-column comparisons
test/cases/15-TagIndices/test_index_tag_basic.py Added test for tag-to-tag comparison and commented out problematic test
test/ci/cases.task Enabled the updated test file in the CI pipeline

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/libs/index/src/indexFilter.c
Comment thread test/cases/15-TagIndices/test_index_tag_basic.py Outdated
Comment thread test/cases/15-TagIndices/test_index_tag_basic.py Outdated
Comment thread source/libs/index/src/indexFilter.c
@guanshengliang
guanshengliang merged commit 4837c27 into main Mar 9, 2026
20 of 24 checks passed
@guanshengliang
guanshengliang deleted the fix/fixTagFilterCrash branch March 9, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants