-
Notifications
You must be signed in to change notification settings - Fork 722
Support multiple columns in fulltext index #24439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
⚪ Test history | Ya make output | Test bloat
🟢 |
} | ||
|
||
// See Layout enum | ||
Layout layout = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не вижу пока причин делать какой-то разный layout для разных колонок, поэтому он на уровне выше
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢 |
There was a problem hiding this 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 modernizes fulltext index configuration by replacing the flat field-based settings structure with a nested column-specific analyzer configuration system.
- Introduces a new
ColumnAnalyzers
message that encapsulates analyzer settings per column - Migrates existing tokenizer, language, and filter settings into an
Analyzers
structure - Updates test cases to use the new configuration format with proper validation for column matching
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
ydb/public/api/protos/ydb_table.proto | Restructures FulltextIndexSettings with column-specific analyzers and moves analyzer settings into nested messages |
ydb/core/tx/schemeshard/ut_index/ut_fulltext_index.cpp | Updates test cases to use new column-based fulltext settings format and adds validation tests |
ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp | Enhances SpecializedIndexDescription to use proper protobuf comparison instead of string comparison |
ydb/core/tx/schemeshard/schemeshard_utils.h | Adds validation logic to ensure column settings match index key columns |
Comments suppressed due to low confidence (3)
ydb/public/api/protos/ydb_table.proto:1
- Method name should be
GetColumns()
instead ofGetcolumns()
. Proto field accessors typically use PascalCase.
syntax = "proto3";
ydb/public/api/protos/ydb_table.proto:1
- Method names should be
GetColumns()
andGetColumn()
instead ofGetcolumns()
andGetcolumn()
. Proto field accessors typically use PascalCase.
syntax = "proto3";
ydb/public/api/protos/ydb_table.proto:1
- Method names should be
GetColumns()
andGetColumn()
instead ofGetcolumns()
andGetcolumn()
. Proto field accessors typically use PascalCase.
syntax = "proto3";
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ |
⚪ Test history | Ya make output | Test bloat
🟢 |
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢 |
Changelog entry
...
Changelog category
Description for reviewers
https://t.me/c/1952638471/9033