Skip to content

fix(core): avoid error when manifest entry has value_stats_cols#150

Merged
lxy-9602 merged 4 commits intoalibaba:mainfrom
SGZW:zhangwei/fix_read
Feb 27, 2026
Merged

fix(core): avoid error when manifest entry has value_stats_cols#150
lxy-9602 merged 4 commits intoalibaba:mainfrom
SGZW:zhangwei/fix_read

Conversation

@SGZW
Copy link
Contributor

@SGZW SGZW commented Feb 26, 2026

Purpose

When a manifest entry contains value_stats_cols but has not enabled the value filter, reading the PK table will throw a NotImplemented error. I've identified that the value filter functionality is actually not implemented in the code; I've temporarily bypassed this issue with a special check and will implement the value_stats_cols filter later.

Tests

API and Format

Documentation

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents KeyValueFileStoreScan from throwing Status::NotImplemented when manifest entries contain value_stats_cols but value filtering is not actually enabled, by short-circuiting whole-bucket filtering paths when the value filter is disabled.

Changes:

  • Add IsValueFilterEnabled() checks to bypass FilterByValueFilter(...) in FilterWholeBucketPerFile.
  • Add IsValueFilterEnabled() checks to bypass FilterByValueFilter(...) in FilterWholeBucketAllFiles.

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

@SGZW SGZW force-pushed the zhangwei/fix_read branch from 2d889b2 to 4764a4e Compare February 26, 2026 07:28
@SGZW SGZW force-pushed the zhangwei/fix_read branch from 9cbdc76 to ebd3a3e Compare February 26, 2026 13:54
@SGZW SGZW force-pushed the zhangwei/fix_read branch from ebd3a3e to 66dcf84 Compare February 27, 2026 02:50
@SGZW SGZW requested a review from lxy-9602 February 27, 2026 03:02
Copy link
Collaborator

@lxy-9602 lxy-9602 left a comment

Choose a reason for hiding this comment

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

+1

@lxy-9602 lxy-9602 merged commit 32aa8ca into alibaba:main Feb 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants