Skip to content

Conversation

@lucasfang
Copy link
Collaborator

Purpose

Linked issue: close #xxx

Tests

API and Format

Documentation

Copilot AI review requested due to automatic review settings January 5, 2026 08:42
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lucasfang lucasfang changed the title Support orc prefetch read Extract interfaces from FileBatchReader to PrefetchFileBatchReader Jan 7, 2026
@lucasfang lucasfang requested a review from Copilot January 7, 2026 03:02
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lucasfang lucasfang requested a review from Copilot January 7, 2026 06:42
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

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/paimon/common/reader/prefetch_file_batch_reader_impl.cpp:107

  • The constructor parameter is passed by const reference but then moved with std::move. This is problematic because:
  1. Moving from a const reference is ineffective - it will copy instead of move
  2. The parameter should either be passed by value (and then moved), or passed by rvalue reference

The constructor should either accept the parameter by value or by rvalue reference to properly transfer ownership.
src/paimon/common/reader/prefetch_file_batch_reader_impl.h:111

  • The constructor parameter is declared as const reference but is moved from with std::move on line 107 of the implementation file. This is ineffective - moving from a const reference results in a copy, not a move. The parameter should either be passed by value or by rvalue reference to enable proper move semantics.

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

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

@lucasfang lucasfang merged commit 67ec385 into alibaba:main Jan 8, 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.

2 participants