Skip to content

Conversation

@sanikolaev
Copy link
Collaborator

@sanikolaev sanikolaev commented Oct 31, 2025

Related issue #3864

@sanikolaev sanikolaev added the pack To trigger CI to build packages for PR label Oct 31, 2025
Copy link
Contributor

@tomatolog tomatolog left a comment

Choose a reason for hiding this comment

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

it could be better to refactor the ProcessCollectedHits function and use it here along with CSphSource::BuildHits as that function could handle blended hits but this function missed this case with the blended. And it could be better to has the single code (ProcessCollectedHits) there all bugs could be fixed at once instead of fix the possible bug at the ProcessCollectedHits then at CSphSource_SQL::IterateJoinedHits

- Moved ProcessCollectedHits to a public method in CSphSource, removing unnecessary parameters.
- Updated references to use member variables directly, improving clarity and maintainability.
- Adjusted logic in SQL source iteration to utilize the new ProcessCollectedHits method for handling blended hits and setting end markers.
…ts sequence

- Introduced FindBlendedHitsStart method to encapsulate logic for finding the start index of the last blended hits sequence in joined field hits.
- Updated IterateJoinedHits to utilize the new method, improving code clarity and maintainability.
@sanikolaev sanikolaev requested a review from tomatolog November 3, 2025 16:04
@sanikolaev
Copy link
Collaborator Author

it could be better to refactor the ProcessCollectedHits function

Done. Pls review.
I've also updated ubertest 146.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

Windows test results

    3 files      3 suites   26m 2s ⏱️
1 112 tests 1 057 ✅ 54 💤 1 ❌
1 114 runs  1 059 ✅ 54 💤 1 ❌

For more details on these failures, see this check.

Results for commit 57c9ac2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pack To trigger CI to build packages for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants