Skip to content

Refactor 0527#63712

Merged
Gabriel39 merged 7 commits into
apache:refact_reader_branchfrom
Gabriel39:refactor_0527
May 27, 2026
Merged

Refactor 0527#63712
Gabriel39 merged 7 commits into
apache:refact_reader_branchfrom
Gabriel39:refactor_0527

Conversation

@Gabriel39
Copy link
Copy Markdown
Contributor

@Gabriel39 Gabriel39 commented May 27, 2026

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

Gabriel39 added 7 commits May 27, 2026 09:25
@Gabriel39
[fix](be) Use conjunct for parquet local filter
d0e4678 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: ParquetReader::_read_filter_columns evaluated localized filters through FileLocalFilter::predicates, while the localized filter expression is stored in FileLocalFilter::conjunct. This changes the lazy filter path to execute the conjunct and intersect its result with the selection vector.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. build-support/clang-format.sh and build-support/check-format.sh could not run because clang-format/llvm@16 is not installed in this environment. Compile test was not run because the existing compile_commands.json points to unavailable /mnt paths and ninja is not installed.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[test](be) Cover parquet conjunct local filter
1533a64 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: NewParquetReaderTest only populated FileLocalFilter::predicates for local predicate filtering. Parquet row group pruning still uses predicates, while row filtering now uses conjunct, so the tests need to populate both with matching semantics.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. ./run-be-ut.sh --run '--filter=NewParquetReaderTest.*' could not run because JAVA_HOME points to JDK 11 and JDK_17 is not set. build-support/check-format.sh could not run because clang-format is not installed.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[fix](be) Include vexpr context for parquet filter
1fde475 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: ParquetReader now calls VExprContext::execute_filter directly, but parquet_reader.cpp only had the forward declaration from vexpr_fwd.h. Include vexpr_context.h so the method definition is visible.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[test](be) Cover table reader split reopen
fa3a7dc 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: TableReader did not have a test that reused the same instance across repeated split lifecycles. Add a test that simulates init() followed by prepare_split(), get_block(), and close() three times, with comments documenting the intended scanner workflow.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. ./run-be-ut.sh --run '--filter=TableReaderTest.*' could not run because JAVA_HOME points to JDK 11 and JDK_17 is not set. build-support/check-format.sh could not run because clang-format is not installed.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[test](be) Cover table reader schema mismatch
5adcb34 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: TableReader split lifecycle coverage did not include the case where TableReadOptions::projected_columns does not match the ParquetReader file-local schema. Add a negative test that requests a missing projected field and verifies get_block() rejects the mismatch when the split is opened lazily.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. ./run-be-ut.sh --run '--filter=TableReaderTest.*' could not run because JAVA_HOME points to JDK 11 and JDK_17 is not set. build-support/check-format.sh could not run because clang-format is not installed.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[test](be) Cover table reader mapper projections
1c9ca7a 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: TableReader tests covered rejected projected_columns mismatches but not schema differences that ColumnMapper can resolve through projection expressions. Add a case where ParquetReader exposes id/value while TableReadOptions requests renamed table columns and an INT-to-BIGINT projection, covering SlotRef and Cast output paths.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. ./run-be-ut.sh --run '--filter=TableReaderTest.*' could not run because JAVA_HOME points to JDK 11 and JDK_17 is not set. build-support/check-format.sh could not run because clang-format is not installed.
- Behavior changed: No
- Does this need documentation: No
@Gabriel39
[test](be) Cover table reader field id mismatch
efdb0e0 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: TableReader schema mismatch coverage did not distinguish same-name columns with different field ids. Add a test proving ColumnMapper in BY_FIELD_ID mode rejects a projected column whose name matches the Parquet field but whose id does not.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Ran git diff --check. ./run-be-ut.sh --run '--filter=TableReaderTest.*' could not run because JAVA_HOME points to JDK 11 and JDK_17 is not set. build-support/check-format.sh could not run because clang-format is not installed.
- Behavior changed: No
- Does this need documentation: No
@hello-stephen
Copy link
Copy Markdown
Contributor

hello-stephen commented May 27, 2026

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@Gabriel39 Gabriel39 merged commit 4fe7254 into apache:refact_reader_branch May 27, 2026
9 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Gabriel39 @hello-stephen