allow to return relevance value as a result #31285
Conversation
Run Extra TestsRun additional tests for this PR. You can customize:
|
|
🟢 |
|
⚪ ⚪ DetailsYa make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Pull request overview
This PR enables returning relevance values as results from full-text search queries. Previously, relevance could only be used internally for sorting; now it can be included in the SELECT clause and returned to users.
Key changes:
- Uncommented and updated test code to verify relevance values can be retrieved in query results
- Refactored full-text search optimization logic by extracting common functionality into reusable helper functions (
FindMatchingApplyandExtractFullTextSearchParams) - Extended
KqpRewriteFlatMapOverFullTextContainsto handle bothFullText.ContainsandFullText.Relevanceoperations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ydb/core/kqp/ut/indexes/kqp_indexes_fulltext_ut.cpp | Uncommented test code that verifies relevance values can be selected and retrieved from full-text search results, added relevance value extraction and updated debug output |
| ydb/core/kqp/opt/logical/kqp_opt_log_indexes.cpp | Refactored full-text search query optimization by extracting common logic into helper functions, added support for returning relevance values in FlatMap operations, and simplified lambda construction logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TMaybeNode<TCoApply> FindMatchingApply(const TExprBase& node) { | ||
| TMaybeNode<TCoApply> matchingApply; | ||
|
|
||
| VisitExpr(node.Ptr(), [&] (const TExprNode::TPtr& expr) { | ||
| if (TCoApply::Match(expr.Get())) { | ||
| auto apply = TExprBase(expr).Cast<TCoApply>(); | ||
| if (!apply.Callable().Maybe<TCoUdf>()) { | ||
| return true; | ||
| } | ||
|
|
||
| if (apply.Args().Count() != 3) { | ||
| return true; | ||
| } | ||
|
|
||
| auto udf = apply.Callable().Maybe<TCoUdf>().Cast(); | ||
|
|
||
| if (IsIn({"FullText.Contains", "FullText.Relevance"}, udf.MethodName().Value())) { | ||
| matchingApply = apply; | ||
| return true; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| return true; | ||
| }); | ||
|
|
||
| return matchingApply; | ||
| } |
There was a problem hiding this comment.
The FindMatchingApply function will return the last matching apply node if multiple FullText.Contains or FullText.Relevance calls exist in the expression tree, due to the visitor overwriting matchingApply on each match. This could lead to unexpected behavior if an expression contains multiple full-text operations. Consider either: (1) stopping after finding the first match by returning false from the visitor, or (2) detecting multiple matches and reporting an error if that's not a valid use case.
gridnevvvit
force-pushed
the
fix-top-sort-without-flat-map
branch
from
ce5d823 to
8377d84
Compare
gridnevvvit
force-pushed
the
fix-top-sort-without-flat-map
branch
from
8377d84 to
b295f0e
Compare
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
2 participants
Changelog entry
Changelog category
Description for reviewers
...