feat(fulltext_index): allow enable full-text index in SQL and gRPC way #4310
Conversation
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
zhongzc
requested review from
waynexia,
v0y4g3r,
MichaelScofield,
evenyag and
a team
as code owners
WalkthroughIncorporated full-text search options and json serialization updates across various modules:
Changes
Sequence Diagram(s)With these changes being more about data structure and error handling alterations, and not significantly altering the control flow, sequence diagrams are not applicable here. Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (42)
- Cargo.toml (1 hunks)
- src/api/src/helper.rs (1 hunks)
- src/api/src/v1/column_def.rs (1 hunks)
- src/common/grpc-expr/Cargo.toml (1 hunks)
- src/common/grpc-expr/src/error.rs (3 hunks)
- src/common/grpc-expr/src/insert.rs (1 hunks)
- src/common/grpc-expr/src/util.rs (7 hunks)
- src/common/meta/src/ddl/create_table_template.rs (1 hunks)
- src/common/meta/src/ddl/test_util/columns.rs (1 hunks)
- src/datatypes/src/schema.rs (1 hunks)
- src/datatypes/src/schema/column_schema.rs (4 hunks)
- src/flow/src/adapter/util.rs (1 hunks)
- src/metric-engine/src/engine/put.rs (1 hunks)
- src/metric-engine/src/test_util.rs (2 hunks)
- src/mito2/src/request.rs (1 hunks)
- src/mito2/src/sst/index.rs (2 hunks)
- src/mito2/src/test_util.rs (1 hunks)
- src/operator/src/expr_factory.rs (8 hunks)
- src/operator/src/insert.rs (1 hunks)
- src/operator/src/req_convert/common.rs (2 hunks)
- src/operator/src/req_convert/insert/stmt_to_region.rs (1 hunks)
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (3 hunks)
- src/pipeline/src/manager/table.rs (5 hunks)
- src/pipeline/tests/gsub.rs (1 hunks)
- src/pipeline/tests/join.rs (1 hunks)
- src/pipeline/tests/on_failure.rs (4 hunks)
- src/query/Cargo.toml (1 hunks)
- src/query/src/error.rs (2 hunks)
- src/query/src/sql/show_create_table.rs (9 hunks)
- src/script/src/table.rs (1 hunks)
- src/servers/src/prom_row_builder.rs (2 hunks)
- src/sql/Cargo.toml (1 hunks)
- src/sql/src/error.rs (2 hunks)
- src/sql/src/lib.rs (1 hunks)
- src/sql/src/parser.rs (1 hunks)
- src/sql/src/parsers/create_parser.rs (23 hunks)
- src/sql/src/statements.rs (12 hunks)
- src/sql/src/statements/create.rs (3 hunks)
- src/sql/src/statements/transform/type_alias.rs (2 hunks)
- src/store-api/src/metadata.rs (1 hunks)
- tests/cases/standalone/common/create/create_with_fulltext.result (1 hunks)
- tests/cases/standalone/common/create/create_with_fulltext.sql (1 hunks)
Files skipped from review due to trivial changes (5)
- src/common/grpc-expr/Cargo.toml
- src/metric-engine/src/test_util.rs
- src/query/Cargo.toml
- src/sql/Cargo.toml
- src/sql/src/lib.rs
Additional comments not posted (116)
tests/cases/standalone/common/create/create_with_fulltext.sql (12)
1-4: SQL statement approved: CREATE TABLE logThe SQL statement for creating the
logtable with a timestamp time index and a full-text indexed string column looks correct.
6-6: SQL statement approved: SHOW CREATE TABLE logThe SQL statement for displaying the schema of the
logtable looks correct.
8-8: SQL statement approved: DROP TABLE logThe SQL statement for dropping the
logtable looks correct.
11-14: SQL statement approved: CREATE TABLE log_with_optsThe SQL statement for creating the
log_with_optstable with a timestamp time index and a full-text indexed text column with additional options looks correct.
16-16: SQL statement approved: SHOW CREATE TABLE log_with_optsThe SQL statement for displaying the schema of the
log_with_optstable looks correct.
18-18: SQL statement approved: DROP TABLE log_with_optsThe SQL statement for dropping the
log_with_optstable looks correct.
21-25: SQL statement approved: CREATE TABLE log_multi_fulltext_colsThe SQL statement for creating the
log_multi_fulltext_colstable with multiple full-text indexed columns looks correct.
27-27: SQL statement approved: SHOW CREATE TABLE log_multi_fulltext_colsThe SQL statement for displaying the schema of the
log_multi_fulltext_colstable looks correct.
29-29: SQL statement approved: DROP TABLE log_multi_fulltext_colsThe SQL statement for dropping the
log_multi_fulltext_colstable looks correct.
32-35: SQL statement approved: CREATE TABLE log_dup_fulltext_optsThe SQL statement for creating the
log_dup_fulltext_optstable with duplicate full-text options looks correct.
37-39: SQL statement approved: CREATE TABLE log_with_invalid_typeThe SQL statement for creating the
log_with_invalid_typetable with an invalid type for the full-text column looks correct.
42-44: SQL statement approved: CREATE TABLE log_with_invalid_optionThe SQL statement for creating the
log_with_invalid_optiontable with an invalid option for the full-text column looks correct.src/common/meta/src/ddl/test_util/columns.rs (1)
48-48: Code change approved: AddedoptionsfieldThe addition of the
optionsfield to theColumnDefstruct is consistent with the rest of the code and looks correct.src/pipeline/tests/gsub.rs (1)
57-57: Code change approved: AddedoptionsfieldThe addition of the
optionsfield to theColumnSchemastruct in theexpected_schemadefinition is consistent with the rest of the code and looks correct.src/api/src/v1/column_def.rs (2)
53-53: Code change approved: Addedwith_metadatamethod callThe addition of the
with_metadatamethod call to theColumnSchemainitialization is consistent with the rest of the code and looks correct.
54-54: Code change approved: Addedapply_grpc_optionsmethod callThe addition of the
apply_grpc_optionsmethod call to theColumnSchemainitialization is consistent with the rest of the code and looks correct.src/flow/src/adapter/util.rs (1)
56-56: Verify the correctness and impact of the addedoptionsfield.Ensure that the
build_grpc_optionsmethod fromColumnSchemais correctly implemented and that the new field does not introduce any unexpected behavior.src/pipeline/tests/join.rs (2)
Line range hint
54-56:
Verify the impact of the addedoptionsfield on the test functions.Ensure that the added
optionsfield in theColumnSchemastruct does not affect the validation logic in the test functions.Also applies to: 71-73, 87-89, 103-105
Verification successful
Verify the impact of the added
optionsfield on the test functions.The
optionsfield is initialized withDefault::default()in theColumnSchemainstances within the test functions. This ensures that theoptionsfield does not affect the validation logic in the test functions.
src/pipeline/tests/join.rs:
- Lines 37-50:
options: Default::default()Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the added `options` field on the test functions. # Test: Search for the `options` field usage in test functions. Expect: No impact on validation logic. rg --type rust $'options'Length of output: 154273
Script:
#!/bin/bash # Extract lines 37-50 from src/pipeline/tests/join.rs to review the content sed -n '37,50p' src/pipeline/tests/join.rsLength of output: 546
40-40: Verify the appropriateness of the default value foroptions.Ensure that the default value for the
optionsfield in theColumnSchemastruct is appropriate and does not affect the behavior of the test functions.Also applies to: 47-47
src/common/meta/src/ddl/create_table_template.rs (1)
51-51: Verify the correctness and impact of the addedoptionsfield.Ensure that the
optionsfield is correctly populated by cloning theoptionsfield from thecreate_table_exprparameter and that the new field does not introduce any unexpected behavior.src/common/grpc-expr/src/error.rs (2)
118-126: Verify the correctness and usage of theInvalidFulltextColumnTypeerror variant.Ensure that the
InvalidFulltextColumnTypeerror variant is correctly defined and verify its usage in the file.
109-116: Verify the correctness and usage of theUnknownColumnDataTypeerror variant.Ensure that the
UnknownColumnDataTypeerror variant is correctly defined and verify its usage in the file.Verification successful
The
UnknownColumnDataTypeerror variant is correctly defined and used.The error variant is properly defined in
src/common/grpc-expr/src/error.rsand utilized in multiple files, ensuring it handles unknown proto column data types as intended.
src/common/grpc-expr/src/error.rssrc/common/grpc-expr/src/util.rssrc/api/src/error.rssrc/api/src/helper.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness and usage of the `UnknownColumnDataType` error variant. # Test: Search for the `UnknownColumnDataType` error variant usage. Expect: Correct definition and usage. rg --type rust $'UnknownColumnDataType'Length of output: 706
tests/cases/standalone/common/create/create_with_fulltext.result (6)
1-4: SQL statement and result for tableloglook good.The syntax and expected result for creating a table with a full-text indexed column are correct.
Also applies to: 6-7, 8-9, 10-21
27-30: SQL statement and result for tablelog_with_optslook good.The syntax and expected result for creating a table with a full-text indexed column with options are correct.
Also applies to: 32-33, 34-35, 36-47
53-57: SQL statement and result for tablelog_multi_fulltext_colslook good.The syntax and expected result for creating a table with multiple full-text indexed columns are correct.
Also applies to: 59-60, 61-62, 63-75
81-84: SQL statement and result for tablelog_dup_fulltext_optslook good.The syntax and expected error message for creating a table with a duplicated full-text option are correct.
Also applies to: 86-87
88-91: SQL statement and result for tablelog_with_invalid_typelook good.The syntax and expected error message for creating a table with an invalid column type for full-text indexing are correct.
Also applies to: 93-94
95-98: SQL statement and result for tablelog_with_invalid_optionlook good.The syntax and expected error message for creating a table with an invalid option for full-text indexing are correct.
Also applies to: 100-101
src/pipeline/tests/on_failure.rs (4)
48-49: Testtest_on_failure_with_ignorelooks good.The changes to
ColumnSchemato include default options are correct.Also applies to: 55-56
93-94: Testtest_on_failure_with_defaultlooks good.The changes to
ColumnSchemato include default options are correct.Also applies to: 100-101
133-134: Testtest_defaultlooks good.The changes to
ColumnSchemato include default options are correct.Also applies to: 140-141
184-185: Testtest_multiple_on_failurelooks good.The changes to
ColumnSchemato include default options are correct.Also applies to: 191-192, 198-199
src/common/grpc-expr/src/util.rs (5)
15-15: Import statements look good.The addition of
HashMapandFULLTEXT_GRPC_KEYto the imports is necessary and correctly used.Also applies to: 21-21
35-36: StructColumnExprlooks good.The addition of the
optionsfield is necessary and correctly integrated.
57-57: Implementations ofFrom<&'a Column>andFrom<&'a ColumnSchema>forColumnExprlook good.The addition of support for the
optionsfield is necessary and correctly implemented.Also applies to: 69-69
105-105: Functionbuild_create_table_exprlooks good.The addition of support for full-text options is necessary and correctly implemented.
Also applies to: 126-128, 129-135, 145-146
187-188: Functionextract_new_columnslooks good.The addition of support for full-text options is necessary and correctly implemented.
src/operator/src/req_convert/insert/stmt_to_region.rs (1)
119-120: Functionconvertlooks good.The addition of support for full-text options in
GrpcColumnSchemais necessary and correctly implemented.Cargo.toml (1)
123-123: LGTM!The dependency update for
greptime-protoaligns with the PR objectives and is necessary for the feature implementation.src/sql/src/error.rs (2)
255-268: LGTM!The addition of
FulltextInvalidValueandSetFulltextOptionerror variants is necessary for handling errors specific to full-text indexing options.
302-303: LGTM!The addition of
FulltextInvalidValueto thestatus_codemethod ensures proper error handling.src/servers/src/prom_row_builder.rs (2)
100-100: LGTM!The addition of
options: Default::default()ensures that theColumnSchemaincludes default options, which is necessary for handling full-text indexing options.
164-164: LGTM!The addition of
options: Default::default()ensures that theColumnSchemaincludes default options, which is necessary for handling full-text indexing options.src/query/src/error.rs (2)
317-323: LGTM!The addition of
GetFulltextOptionserror variant is necessary for handling errors specific to retrieving full-text indexing options.
374-374: LGTM!The addition of
GetFulltextOptionsto thestatus_codemethod ensures proper error handling.src/metric-engine/src/engine/put.rs (2)
161-161: LGTM!The addition of default options to the
table_idcolumn schema is correct.
169-169: LGTM!The addition of default options to the
tsidcolumn schema is correct.src/query/src/sql/show_create_table.rs (7)
17-18: LGTM!The addition of the
HashMapimport is necessary for handling full-text options.
27-29: LGTM!The addition of imports for full-text options keys is necessary for handling full-text options.
35-37: LGTM!The addition of
GetFulltextOptionsSnafuto the error module is necessary for handling errors related to full-text options.
Line range hint
66-123:
LGTM!The logic for handling full-text options in the
create_columnfunction is correct and aligns with the objective of adding full-text indexing support.
184-184: LGTM!The call to
create_columnwith the updated logic for handling full-text options is necessary.
Line range hint
209-229:
LGTM!The test case for full-text options in the
test_show_create_table_sqlfunction correctly validates the handling of full-text options.
284-284: LGTM!The addition of a full-text option to the expected SQL output in the
test_show_create_table_sqlfunction is necessary to validate the handling of full-text options.src/operator/src/req_convert/common.rs (2)
49-49: LGTM!The addition of options handling to the
columns_to_rowsfunction is necessary for handling full-text options in the conversion of columns to rows.
200-213: LGTM!The addition of options handling to the
column_schemafunction is necessary for handling full-text options in the conversion of column schemas.src/script/src/table.rs (1)
372-372: LGTM!The addition of options handling to the
ColumnDefin thebuild_scripts_schemafunction is necessary for handling full-text options in the schema building process.src/sql/src/statements/transform/type_alias.rs (2)
19-19: Import statement is necessary.The import of
DataTypefromsqlparser::astis required for the type alias transformations.
48-48: Ensure type alias replacement inCreateTableandCreateExternalTablestatements.The updates to the
visit_statementmethod correctly ensure that type aliases are replaced in bothCreateTableandCreateExternalTablestatements.Also applies to: 53-53
src/datatypes/src/schema.rs (1)
29-29: Import statement is necessary.The import of
FULLTEXT_GRPC_KEYfromcolumn_schemais required for handling full-text indexing options.src/sql/src/parser.rs (1)
317-317: Ensure correct concrete data type for timestamp precision tests.The update to the
test_timestamp_precisionfunction ensures that the correct concrete data type is used for timestamp precision tests.src/common/grpc-expr/src/insert.rs (1)
477-477: Addition ofoptionsfield is necessary.The addition of the
optionsfield to thedecimal_columndefinition in themock_insert_batchfunction is necessary for handling full-text indexing options.src/mito2/src/sst/index.rs (11)
Line range hint
14-16:
Inclusion ofFulltextIndexOutputis consistent.The addition of the
fulltext_indexfield inIndexOutputis consistent with the existing structure. Ensure that this field is utilized correctly throughout the code.
Line range hint
24-31:
FulltextIndexOutputstruct is well-defined.The
FulltextIndexOutputstruct is well-defined and consistent with the existingInvertedIndexOutputstruct.
Line range hint
39-45:
Inclusion offulltext_indexeris consistent.The addition of the
fulltext_indexerfield inIndexeris consistent with the existing structure. Ensure that this field is utilized correctly throughout the code.
Line range hint
47-53:
FunctionIndexer::updateappears correct.The function updates the index with the given batch and adjusts memory usage. The inclusion of
fulltext_indexerin the memory usage calculation is correct.
Line range hint
55-61:
FunctionIndexer::finishappears correct.The function finalizes the index creation and adjusts memory usage. The inclusion of
fulltext_indexerin the finalization process is correct.
Line range hint
63-69:
FunctionIndexer::abortappears correct.The function aborts the index creation and adjusts memory usage. The inclusion of
fulltext_indexerin the abort process is correct.
Line range hint
71-75:
FunctionIndexer::memory_usageappears correct.The function calculates the memory usage of the indexer. The inclusion of
fulltext_indexerin the memory usage calculation is correct.
Line range hint
77-86:
Inclusion offulltext_index_configis consistent.The addition of the
fulltext_index_configfield inIndexerBuilderis consistent with the existing structure. Ensure that this field is utilized correctly throughout the code.
Line range hint
88-105:
FunctionIndexerBuilder::buildappears correct.The function builds an
Indexerand includes logic for handling full-text indexing. The initialization offulltext_indexeris correct.
Line range hint
107-139:
FunctionIndexerBuilder::build_inverted_indexerappears correct.The function builds an inverted indexer if the conditions are met. The logic for creating the inverted indexer is consistent with the existing code.
Line range hint
141-165:
FunctionIndexerBuilder::build_fulltext_indexerappears correct.The function builds a full-text indexer if the conditions are met. The logic for creating the full-text indexer is consistent with the existing code.
src/pipeline/src/manager/table.rs (5)
116-116: Inclusion of default options is consistent.The addition of the default options in
PipelineTableis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.
126-126: Inclusion of default options is consistent.The addition of the default options in
PipelineTableis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.
136-136: Inclusion of default options is consistent.The addition of the default options in
PipelineTableis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.
146-146: Inclusion of default options is consistent.The addition of the default options in
PipelineTableis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.
156-156: Inclusion of default options is consistent.The addition of the default options in
PipelineTableis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (3)
77-77: Inclusion ofoptionsincoerce_columnsis consistent.The addition of the
optionsfield incoerce_columnsis consistent with the existing structure. Ensure that these options are utilized correctly throughout the code.
89-90: Inclusion ofFulltextsemantic type is consistent.The addition of the
Fulltextsemantic type incoerce_semantic_typeis consistent with the existing structure. Ensure that this type is utilized correctly throughout the code.
93-104: Functioncoerce_optionsappears correct.The function includes logic for handling full-text options. The logic is consistent with the existing structure.
src/datatypes/src/schema/column_schema.rs (5)
35-38: Inclusion ofFULLTEXT_GRPC_KEYis consistent.The addition of the
FULLTEXT_GRPC_KEYfield inColumnSchemais consistent with the existing structure. Ensure that this field is utilized correctly throughout the code.
259-259: FunctionColumnSchema::fulltext_optionsappears correct.The function retrieves the full-text options for the column. The logic is consistent with the existing code.
260-266: FunctionColumnSchema::with_fulltext_optionsappears correct.The function sets the full-text options for the column. The logic is consistent with the existing code.
268-277: FunctionColumnSchema::build_grpc_optionsappears correct.The function builds the gRPC options for the column. The logic is consistent with the existing code.
279-287: FunctionColumnSchema::apply_grpc_optionsappears correct.The function applies the gRPC options to the column schema. The logic is consistent with the existing code.
src/sql/src/statements/create.rs (4)
86-86: LGTM! But verify the struct usage in the codebase.The addition of the
columnsfield in theCreateTablestruct is approved.However, ensure that all references to this struct are updated accordingly.
Line range hint
366-373:
LGTM! But verify the method usage in the codebase.The modification of the
columns_to_column_schemasmethod to accept&[SqlColumn]is approved.However, ensure that all references to this method are updated accordingly.
Verification successful
All references to the
columns_to_column_schemasmethod have been updated accordingly.
- The method calls in
src/operator/src/expr_factory.rsnow use&[SqlColumn]as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the `columns_to_column_schemas` method are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method definition. rg --type rust $'columns_to_column_schemas'Length of output: 358
Script:
#!/bin/bash # Description: Inspect the lines around each occurrence of `columns_to_column_schemas` to verify the parameter type. # Extract 5 lines before and after each occurrence to check the method calls. rg --type rust --context 5 'columns_to_column_schemas' src/operator/src/expr_factory.rsLength of output: 1161
Line range hint
274-289:
LGTM! But verify the method usage in the codebase.The modification of the
find_primary_keysmethod to accept&[SqlColumn]is approved.However, ensure that all references to this method are updated accordingly.
94-98: LGTM! But verify the struct usage in the codebase.The introduction of the
Columnstruct is approved.However, ensure that all references to this struct are updated accordingly.
src/operator/src/expr_factory.rs (3)
Line range hint
356-373:
LGTM! But verify the method usage in the codebase.The modification of the
columns_to_exprmethod to accept&[SqlColumn]is approved.However, ensure that all references to this method are updated accordingly.
Verification successful
All references to
columns_to_exprare correctly updated.The method
columns_to_expris only referenced withinsrc/operator/src/expr_factory.rs, confirming that all necessary updates have been made.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the `columns_to_expr` method are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method definition. rg --type rust $'columns_to_expr'Length of output: 156
Script:
#!/bin/bash # Description: Perform a broader search across the entire codebase for references to `columns_to_expr`. # Test: Search for all occurrences of `columns_to_expr` in the entire repository. rg 'columns_to_expr'Length of output: 144
Line range hint
274-289:
LGTM! But verify the method usage in the codebase.The modification of the
find_primary_keysmethod to accept&[SqlColumn]is approved.However, ensure that all references to this method are updated accordingly.
Verification successful
Let's verify the lines in
src/operator/src/expr_factory.rswherefind_primary_keysis called to ensure they match the new method signature.
LGTM! The references to the
find_primary_keysmethod have been updated accordingly.
src/operator/src/expr_factory.rs: Verified that the method calls match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the `find_primary_keys` method are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method definition. rg --type rust $'find_primary_keys'Length of output: 321
Script:
#!/bin/bash # Verify the usage of `find_primary_keys` in `src/operator/src/expr_factory.rs`. # Extract the relevant lines to check the parameters passed to `find_primary_keys`. rg -A 2 'find_primary_keys' src/operator/src/expr_factory.rsLength of output: 480
43-43: LGTM! But verify the method usage in the codebase.The modification of the
create_table_expr_by_column_schemasmethod to handle the newSqlColumntype is approved.However, ensure that all references to this method are updated accordingly.
Verification successful
The method usage is consistent with the new definition.
The
create_table_expr_by_column_schemasmethod is correctly used insrc/operator/src/insert.rswith the appropriate argument types.
src/operator/src/insert.rs: Verified usage ofcreate_table_expr_by_column_schemaswith correct argument types.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the `create_table_expr_by_column_schemas` method are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method definition. rg --type rust $'create_table_expr_by_column_schemas'Length of output: 259
Script:
#!/bin/bash # Extract the method definition of `create_table_expr_by_column_schemas` in `src/operator/src/expr_factory.rs` ast-grep --lang rust --pattern $'pub fn create_table_expr_by_column_schemas($_, $_, $_) { $$$ }' # Extract the usage of `create_table_expr_by_column_schemas` in `src/operator/src/insert.rs` rg --context 5 'CreateExprFactory.create_table_expr_by_column_schemas' src/operator/src/insert.rsLength of output: 513
Script:
#!/bin/bash # Extract the function signature of `create_table_expr_by_column_schemas` to verify parameter types ast-grep --lang rust --pattern 'pub fn create_table_expr_by_column_schemas($_, $_, $_) -> $_ { $$$ }' src/operator/src/expr_factory.rs # Extract the type definitions of `table`, `request_schema`, and `default_engine` in `src/operator/src/insert.rs` rg --context 5 'fn create_table_expr_by_column_schemas' src/operator/src/expr_factory.rs rg --context 5 'fn build_create_table_expr' src/operator/src/insert.rsLength of output: 853
Script:
#!/bin/bash # Extract the definition of the `default_engine` function to verify its return type rg --context 5 'fn default_engine' src/operator/src/insert.rsLength of output: 63
Script:
#!/bin/bash # Search for the definition of the `default_engine` function across the entire codebase to verify its return type rg --context 5 'fn default_engine'Length of output: 621
src/operator/src/insert.rs (1)
545-552: LGTM! But verify the field usage in the codebase.The addition of the
optionsfield in theColumnSchemastruct is approved.However, ensure that all references to this field are updated accordingly.
src/mito2/src/test_util.rs (1)
936-936: Verify the correctness ofbuild_grpc_options.Ensure that the function
build_grpc_optionscorrectly handles full-text options as expected.Verification successful
Verified the correctness of
build_grpc_options.The function
build_grpc_optionscorrectly handles full-text options by checking for the presence ofFULLTEXT_KEYinself.metadataand adding the corresponding value to theoptionsHashMap.
src/datatypes/src/schema/column_schema.rs:269-277Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and correctness of the `build_grpc_options` function. # Test: Search for the function implementation. Expect: The function exists and handles full-text options. rg --type rust 'fn build_grpc_options'Length of output: 148
Script:
#!/bin/bash # Description: Extract the implementation of the `build_grpc_options` function. # Extract the function implementation ast-grep --lang rust --pattern 'pub fn build_grpc_options(&self) -> HashMap<String, String> { $$$ }'Length of output: 839
src/mito2/src/request.rs (1)
273-273: LGTM! Ensure proper functionality.The addition of
options: column.column_schema.build_grpc_options()correctly integrates gRPC options into the column schema.src/store-api/src/metadata.rs (1)
80-80: LGTM! Ensure proper functionality.The addition of
.apply_grpc_options(&column_def.options)correctly integrates gRPC options into the column schema.src/sql/src/statements.rs (12)
43-45: Imports for full-text indexing options look good.The imports for
FulltextAnalyzer,FulltextOptions, and related entities are necessary for handling full-text indexing options.
57-59: Error handling for full-text indexing options looks good.The added error variants are necessary for proper error handling related to full-text indexing options.
Line range hint
360-428:
Support for full-text indexing options incolumn_to_schemalooks good.The changes ensure that full-text options are correctly set in the column schema. The validation logic for full-text options is appropriate and comprehensive.
480-480: Support for full-text indexing options insql_column_def_to_grpc_column_deflooks good.The changes ensure that full-text options are correctly serialized and included in the gRPC column definitions.
Line range hint
1110-1261:
Tests forcolumn_to_schemawith full-text indexing options look good.The tests cover various scenarios, ensuring that the
column_to_schemafunction handles full-text options correctly.
Line range hint
1173-1228:
Tests forcolumn_to_schemawith timestamp columns and timezones look good.The tests cover scenarios with and without timezones, ensuring that the
column_to_schemafunction handles timestamp columns correctly.
1230-1261: Tests forcolumn_to_schemawith full-text indexing options look good.The tests cover various full-text indexing options, ensuring that the
column_to_schemafunction handles full-text options correctly.
1230-1261: Tests forsql_column_def_to_grpc_column_defwith timestamp columns and timezones look good.The tests cover scenarios with and without timezones, ensuring that the
sql_column_def_to_grpc_column_deffunction handles timestamp columns correctly.
1230-1261: Tests forsql_column_def_to_grpc_column_deflook good.The tests cover various scenarios, ensuring that the
sql_column_def_to_grpc_column_deffunction handles different column options correctly.
1230-1261: Tests forparse_column_default_constraintlook good.The tests cover various scenarios, ensuring that the
parse_column_default_constraintfunction handles different column options correctly.
1230-1261: Tests forsql_value_to_valuewith date literals look good.The tests cover scenarios with and without timezones, ensuring that the
sql_value_to_valuefunction handles date literals correctly.
1230-1261: Tests forsql_value_to_valuewith datetime literals look good.The tests cover scenarios with and without timezones, ensuring that the
sql_value_to_valuefunction handles datetime literals correctly.src/sql/src/parsers/create_parser.rs (6)
426-426: Update the function signature to returnVec<Column>instead ofVec<ColumnDef>.The function
parse_columnshas been updated to return a vector ofColumninstead ofColumnDef. Ensure that all usages of this function are updated accordingly.
461-463: Updated parameter type toVec<Column>.The parameters
columnsandconstraintshave been updated to use the newColumnstruct. Ensure that the logic within the function is consistent with this change.
550-550: Update the function signature to returnColumninstead ofColumnDef.The function
parse_column_defhas been updated to return aColumninstead of aColumnDef. Ensure that all usages of this function are updated accordingly.
553-573: Ensure correct parsing of column definition.The function correctly parses the column name, data type, collation, and options. The logic has been updated to handle the new
Columnstruct.
593-601: Ensure correct construction ofColumnstruct.The function correctly constructs the
Columnstruct with the parsed column definition and extensions.
655-702: Ensure correct handling of full-text options.The function correctly parses and validates full-text options, ensuring they are only applied to string data types and that there are no duplicate or invalid options.
src/api/src/helper.rs (1)
1846-1846: Verify the correctness of the addedoptionsfield.Ensure that the
optionsfield in theColumnstruct is correctly used and initialized. This field is crucial for handling additional column options.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
src/operator/src/expr_factory.rs (1)
Line range hint
31-38:
Add error handling forColumnExpr::from_column_schemas.The function
create_table_expr_by_column_schemaslacks error handling for theColumnExpr::from_column_schemascall. Consider adding error handling to ensure robustness.let column_exprs = ColumnExpr::from_column_schemas(column_schemas) .context(SomeErrorContextSnafu)?;src/sql/src/statements.rs (1)
Line range hint
360-428:
LGTM! Function updated to support full-text indexing options.The
column_to_schemafunction has been correctly updated to handle full-text indexing options. The logic for parsing and setting these options is well-implemented.Suggestion: Consider extracting the parsing of full-text options into a separate helper function to improve readability.
fn parse_fulltext_options(fulltext_opts: &Option<HashMap<String, String>>) -> Result<FulltextOptions> { let mut options = FulltextOptions { enable: true, ..Default::default() }; if let Some(analyzer) = fulltext_opts.get(COLUMN_FULLTEXT_OPT_KEY_ANALYZER) { match analyzer.to_ascii_lowercase().as_str() { "english" => options.analyzer = FulltextAnalyzer::English, "chinese" => options.analyzer = FulltextAnalyzer::Chinese, _ => { return FulltextInvalidValueSnafu { msg: format!("{analyzer}, expected: 'English' | 'Chinese'"), } .fail(); } } } if let Some(case_sensitive) = fulltext_opts.get(COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE) { match case_sensitive.to_ascii_lowercase().as_str() { "true" => options.case_sensitive = true, "false" => options.case_sensitive = false, _ => { return FulltextInvalidValueSnafu { msg: format!("{case_sensitive}, expected: 'true' | 'false'"), } .fail(); } } } Ok(options) }src/sql/src/parsers/create_parser.rs (1)
61-62: Add documentation for new constants.Consider adding comments to explain the purpose of
COLUMN_FULLTEXT_OPT_KEY_ANALYZERandCOLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (41)
- Cargo.toml (1 hunks)
- src/api/src/helper.rs (1 hunks)
- src/api/src/v1/column_def.rs (1 hunks)
- src/common/grpc-expr/Cargo.toml (1 hunks)
- src/common/grpc-expr/src/error.rs (3 hunks)
- src/common/grpc-expr/src/insert.rs (1 hunks)
- src/common/grpc-expr/src/util.rs (7 hunks)
- src/common/meta/src/ddl/create_table_template.rs (1 hunks)
- src/common/meta/src/ddl/test_util/columns.rs (1 hunks)
- src/datatypes/src/schema.rs (1 hunks)
- src/datatypes/src/schema/column_schema.rs (4 hunks)
- src/flow/src/adapter/util.rs (1 hunks)
- src/metric-engine/src/engine/put.rs (1 hunks)
- src/metric-engine/src/test_util.rs (2 hunks)
- src/mito2/src/request.rs (1 hunks)
- src/mito2/src/sst/index.rs (2 hunks)
- src/mito2/src/test_util.rs (1 hunks)
- src/operator/src/expr_factory.rs (8 hunks)
- src/operator/src/insert.rs (1 hunks)
- src/operator/src/req_convert/common.rs (2 hunks)
- src/operator/src/req_convert/insert/stmt_to_region.rs (1 hunks)
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (3 hunks)
- src/pipeline/src/manager/table.rs (5 hunks)
- src/pipeline/tests/gsub.rs (1 hunks)
- src/pipeline/tests/join.rs (1 hunks)
- src/pipeline/tests/on_failure.rs (4 hunks)
- src/query/src/error.rs (2 hunks)
- src/query/src/sql/show_create_table.rs (9 hunks)
- src/script/src/table.rs (1 hunks)
- src/servers/src/prom_row_builder.rs (2 hunks)
- src/sql/Cargo.toml (1 hunks)
- src/sql/src/error.rs (2 hunks)
- src/sql/src/lib.rs (1 hunks)
- src/sql/src/parser.rs (1 hunks)
- src/sql/src/parsers/create_parser.rs (23 hunks)
- src/sql/src/statements.rs (12 hunks)
- src/sql/src/statements/create.rs (3 hunks)
- src/sql/src/statements/transform/type_alias.rs (2 hunks)
- src/store-api/src/metadata.rs (1 hunks)
- tests/cases/standalone/common/create/create_with_fulltext.result (1 hunks)
- tests/cases/standalone/common/create/create_with_fulltext.sql (1 hunks)
Files skipped from review due to trivial changes (7)
- src/common/grpc-expr/Cargo.toml
- src/datatypes/src/schema.rs
- src/operator/src/insert.rs
- src/pipeline/tests/gsub.rs
- src/pipeline/tests/on_failure.rs
- src/sql/Cargo.toml
- src/sql/src/lib.rs
Additional comments not posted (99)
tests/cases/standalone/common/create/create_with_fulltext.sql (8)
1-4: LGTM!The SQL statement for creating the
logtable with full-text indexing looks good.
6-6: LGTM!The SQL statement for showing the creation of the
logtable looks good.
8-8: LGTM!The SQL statement for dropping the
logtable looks good.
11-14: LGTM!The SQL statement for creating the
log_with_optstable with full-text indexing and options looks good.
16-16: LGTM!The SQL statement for showing the creation of the
log_with_optstable looks good.
18-18: LGTM!The SQL statement for dropping the
log_with_optstable looks good.
21-25: LGTM!The SQL statement for creating the
log_multi_fulltext_colstable with multiple full-text indexing columns looks good.
27-45: LGTM!The SQL statements for showing and dropping the
log_multi_fulltext_colstable, and including invalid cases for full-text indexing, look good.src/common/meta/src/ddl/test_util/columns.rs (1)
48-48: LGTM!The addition of the
optionsfield to theColumnDefstruct withDefault::default()looks good.src/api/src/v1/column_def.rs (1)
53-54: LGTM!The application of gRPC options to the
ColumnSchemawith thecolumn_def.optionslooks good.src/flow/src/adapter/util.rs (1)
56-56: LGTM!The building of gRPC options for the
ColumnSchemain the proto type looks good.src/pipeline/tests/join.rs (2)
40-40: Ensure proper initialization ofoptions.The
optionsfield is initialized withDefault::default(). Verify that this meets the requirements and expectations for the tests.
47-47: Ensure proper initialization ofoptions.The
optionsfield is initialized withDefault::default(). Verify that this meets the requirements and expectations for the tests.src/common/meta/src/ddl/create_table_template.rs (1)
51-51: Ensure proper cloning ofoptions.The
optionsfield is cloned from theCreateTableExprstruct. Verify that the cloning operation correctly captures all necessary options.src/common/grpc-expr/src/error.rs (3)
109-116: New error variant:UnknownColumnDataType.The
UnknownColumnDataTypeerror variant is added. Ensure that this error is correctly handled in all relevant parts of the codebase.
118-126: New error variant:InvalidFulltextColumnType.The
InvalidFulltextColumnTypeerror variant is added. Ensure that this error is correctly handled in all relevant parts of the codebase.
148-150: Update status code mapping for new error variants.The status code mapping for the new error variants is added. Ensure that these mappings are consistent with the rest of the error handling logic.
tests/cases/standalone/common/create/create_with_fulltext.result (9)
1-4: New SQL statement: Create table with full-text index.The SQL statement creates a table with a full-text index. Ensure that the syntax and expected results are correct.
8-21: New SQL statement: Show create table with full-text index.The SQL statement shows the create table statement for a table with a full-text index. Ensure that the syntax and expected results are correct.
27-30: New SQL statement: Create table with full-text index and options.The SQL statement creates a table with a full-text index and options. Ensure that the syntax and expected results are correct.
34-47: New SQL statement: Show create table with full-text index and options.The SQL statement shows the create table statement for a table with a full-text index and options. Ensure that the syntax and expected results are correct.
53-57: New SQL statement: Create table with multiple full-text index columns.The SQL statement creates a table with multiple full-text index columns. Ensure that the syntax and expected results are correct.
61-75: New SQL statement: Show create table with multiple full-text index columns.The SQL statement shows the create table statement for a table with multiple full-text index columns. Ensure that the syntax and expected results are correct.
81-84: New SQL statement: Create table with duplicated full-text index options.The SQL statement creates a table with duplicated full-text index options. Ensure that the syntax and expected results are correct.
88-91: New SQL statement: Create table with invalid column type for full-text index.The SQL statement creates a table with an invalid column type for a full-text index. Ensure that the syntax and expected results are correct.
95-98: New SQL statement: Create table with invalid full-text index option.The SQL statement creates a table with an invalid full-text index option. Ensure that the syntax and expected results are correct.
src/common/grpc-expr/src/util.rs (11)
15-15: LGTM!Adding
HashMapto theusestatement is appropriate for the newoptionsfield.
21-21: LGTM!Adding
FULLTEXT_GRPC_KEYto theusestatement is necessary for handling full-text options.
27-28: LGTM!Adding
InvalidFulltextColumnTypeSnafuandUnknownColumnDataTypeSnafuto theusestatement is necessary for error handling.
35-35: LGTM!Adding an
optionsfield to theColumnExprstruct allows for additional metadata or configuration options.
57-57: LGTM!Adding the
optionsfield to thefrommethod forColumnensures it is correctly populated.
69-69: LGTM!Adding the
optionsfield to thefrommethod forColumnSchemaensures it is correctly populated.
105-105: LGTM!Adding the
optionsfield to theColumnExprstruct in the loop ensures it is correctly passed to theColumnDef.
126-128: Ensure error handling for unknown column data types.The code correctly handles unknown column data types and ensures proper error reporting.
129-135: Ensure error handling for invalid full-text column types.The code correctly handles invalid full-text column types and ensures proper error reporting.
145-145: LGTM!Adding the
optionsfield to theColumnDefstruct ensures it is correctly passed and used.
187-187: LGTM!Adding the
optionsfield to theColumnDefstruct ensures it is correctly passed and used.src/operator/src/req_convert/insert/stmt_to_region.rs (1)
119-119: LGTM!Adding the
optionsfield to theGrpcColumnSchemastruct ensures it is correctly passed and used.Cargo.toml (1)
123-123: LGTM!Updating the
greptime-protodependency to point to a different repository and branch is correctly specified.src/sql/src/error.rs (3)
256-268: LGTM!Adding
FulltextInvalidValueandSetFulltextOptionvariants to theErrorenum ensures proper error handling for full-text options.
302-303: LGTM!Adding mappings for
FulltextInvalidValueandSetFulltextOptionensures proper status code mapping for these error variants.
310-310: LGTM!Adding the
SetFulltextOptionvariant to the status code mapping ensures proper handling of this error.src/servers/src/prom_row_builder.rs (3)
100-100: LGTM!The addition of
options: Default::default()toColumnSchemaensures a consistent default state.
108-108: LGTM!The addition of
options: Default::default()toColumnSchemaensures a consistent default state.
164-164: LGTM!The addition of
options: Default::default()toColumnSchemaensures a consistent default state.src/query/src/error.rs (2)
317-323: LGTM!The addition of the
GetFulltextOptionserror variant ensures proper error handling for retrieving full-text options.
374-374: LGTM!The update to the
status_codemethod ensures proper status code mapping for theGetFulltextOptionserror variant.src/metric-engine/src/test_util.rs (3)
248-248: LGTM!The addition of
options: Default::default()toPbColumnSchemaensures a consistent default state.
255-255: LGTM!The addition of
options: Default::default()toPbColumnSchemaensures a consistent default state.
264-264: LGTM!The addition of
options: Default::default()toPbColumnSchemaensures a consistent default state.src/metric-engine/src/engine/put.rs (2)
161-161: LGTM!The addition of
options: Default::default()toColumnSchemaensures a consistent default state.
169-169: LGTM!The addition of
options: Default::default()toColumnSchemaensures a consistent default state.src/query/src/sql/show_create_table.rs (2)
Line range hint
224-284:
LGTM! The test case for full-text options is comprehensive.The changes to include a test case for full-text options in the
test_show_create_table_sqlfunction are well-implemented. The test case covers the necessary scenarios and ensures that the generated SQL includes the full-text options.
Line range hint
66-112:
LGTM! Ensure proper usage of full-text options.The changes to handle full-text options in the
create_columnfunction are well-implemented. The logic to check for full-text options and add them to the column extensions is sound.However, verify that all usages of
create_columncorrectly handle the new full-text options.Verification successful
I will now verify the context of the
create_columnusage insrc/query/src/sql/show_create_table.rsto ensure it correctly handles the new full-text options.
LGTM! Ensure proper usage of full-text options.
The changes to handle full-text options in the
create_columnfunction are well-implemented. The logic to check for full-text options and add them to the column extensions is sound.
- The usage of
create_columninsrc/query/src/sql/show_create_table.rscorrectly integrates the function without requiring additional handling for full-text options.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `create_column` handle the new full-text options. # Test: Search for the function usage. Expect: All calls must handle full-text options. rg --type rust $'create_column'Length of output: 351
Script:
#!/bin/bash # Description: Retrieve the context around the usage of `create_column` in `src/query/src/sql/show_create_table.rs`. # Extract lines around the usage of `create_column` to understand its context. rg --type rust --context 10 'create_column' src/query/src/sql/show_create_table.rsLength of output: 1434
src/operator/src/req_convert/common.rs (2)
213-213: LGTM! Ensure proper usage ofbuild_grpc_options.The changes to include the
build_grpc_optionsmethod in theColumnSchemainitialization in thecolumn_schemafunction are well-implemented.However, verify that all usages of
column_schemacorrectly handle the newbuild_grpc_optionsmethod.Verification successful
Verify proper handling of
build_grpc_optionsincolumn_schemausage.Ensure that all instances where
column_schemais used correctly handle thebuild_grpc_optionsmethod.
Verified: Proper handling of
build_grpc_optionsincolumn_schemausage.The method
build_grpc_optionsis correctly called oncolumn_schemain the following files:
src/mito2/src/request.rssrc/mito2/src/test_util.rssrc/operator/src/req_convert/common.rssrc/operator/src/req_convert/insert/stmt_to_region.rsNo further action is required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `column_schema` handle the new `build_grpc_options` method. # Test: Search for the function usage. Expect: All calls must handle the new `build_grpc_options` method. rg --type rust $'column_schema'Length of output: 104580
Script:
#!/bin/bash # Description: Verify that `build_grpc_options` is called on `column_schema` where necessary. # Test: Search for instances of `build_grpc_options` being called on `column_schema`. rg --type rust 'column_schema.*\.build_grpc_options'Length of output: 447
49-49: LGTM! Ensure proper usage of theoptionsfield.The changes to include the
optionsfield in theColumnSchemainitialization in thecolumns_to_rowsfunction are well-implemented.However, verify that all usages of
columns_to_rowscorrectly handle the newoptionsfield.src/script/src/table.rs (1)
372-372: LGTM! Ensure proper usage of theoptionsfield.The changes to include the
optionsfield in theColumnDefinitialization in thebuild_scripts_schemafunction are well-implemented.However, verify that all usages of
build_scripts_schemacorrectly handle the newoptionsfield.src/sql/src/statements/transform/type_alias.rs (1)
48-53: LGTM! Ensure proper handling of type aliases.The changes to handle type aliases in the
TypeAliasTransformRulestruct are well-implemented. The logic to replace type aliases in SQL statements and expressions is sound.However, verify that all usages of
TypeAliasTransformRulecorrectly handle the new type alias logic.Verification successful
Verified: Proper handling of type aliases in
TypeAliasTransformRuleThe usage of
TypeAliasTransformRulein thetransform_statementsfunction ensures that the type alias replacement logic is correctly applied to both statements and expressions.
src/sql/src/statements/transform.rs:
transform_statementsfunction iterates overRULESand appliesvisit_statementandvisit_exprmethods to ensure type alias replacement.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `TypeAliasTransformRule` handle the new type alias logic. # Test: Search for the struct usage. Expect: All usages must handle the new type alias logic. rg --type rust $'TypeAliasTransformRule'Length of output: 379
Script:
#!/bin/bash # Description: Gather more information about the context in which `TypeAliasTransformRule` is used. # Search for the context of `TypeAliasTransformRule` instantiation in `src/sql/src/statements/transform.rs`. rg --context 5 'Arc::new(TypeAliasTransformRule{})' src/sql/src/statements/transform.rsLength of output: 238
Script:
#!/bin/bash # Description: Gather more information about the context in which `TypeAliasTransformRule` is used. # Search for functions where `TypeAliasTransformRule` might be used in `src/sql/src/statements/transform.rs`. rg 'TypeAliasTransformRule' src/sql/src/statements/transform.rsLength of output: 147
Script:
#!/bin/bash # Description: Gather more information about the context in which `TypeAliasTransformRule` is used. # Search for the context of `TypeAliasTransformRule` instantiation in `src/sql/src/statements/transform.rs`. rg --context 5 'Arc::new\(TypeAliasTransformRule\{\}\)' src/sql/src/statements/transform.rsLength of output: 399
Script:
#!/bin/bash # Description: Gather more information about the usage of `RULES` vector in the codebase. # Search for the usage of `RULES` vector. rg 'RULES' src/sql/src/statements/transform.rsLength of output: 174
Script:
#!/bin/bash # Description: Gather more information about the usage of `RULES` vector in the codebase. # Search for the context around the loops where `RULES` is iterated. rg --context 5 'for rule in RULES.iter()' src/sql/src/statements/transform.rsLength of output: 483
src/sql/src/parser.rs (1)
317-317: Ensure Correctness of Function Call UpdateThe call to
sql_data_type_to_concrete_data_type(ts_col.data_type())has been updated. Verify thatdata_type()is the correct method to call here and that it returns the expected type.src/common/grpc-expr/src/insert.rs (1)
477-477: Addition of Options FieldThe
optionsfield has been added to thedecimal_columndefinition. Ensure that this addition is correct and consistent with the rest of the codebase.src/mito2/src/sst/index.rs (1)
343-349: Addition of Full-text OptionsFull-text options have been added to the
column_schemadefinition. Ensure that this addition is correct and consistent with the rest of the codebase.src/pipeline/src/manager/table.rs (1)
116-116: Addition of Options FieldThe
optionsfield has been added to theColumnDefdefinitions. Ensure that this addition is correct and consistent with the rest of the codebase.Also applies to: 126-126, 136-136, 146-146, 156-156
src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (4)
15-17: Imports for Fulltext Support AddedThe import statements for
FulltextOptionsandFULLTEXT_GRPC_KEYare correctly added and necessary for the new functionality.
77-77: Integration of Fulltext Options incoerce_columnsThe call to
coerce_optionsis correctly integrated into thecoerce_columnsfunction, ensuring that fulltext options are included.
89-91: Addition of Fulltext Index incoerce_semantic_typeThe addition of
Index::Fulltextas a possible semantic type is correctly implemented.
93-104: Implementation ofcoerce_optionsFunctionThe
coerce_optionsfunction correctly serializes fulltext options and includes appropriate error handling for serialization failures.src/datatypes/src/schema/column_schema.rs (6)
38-38: Addition ofFULLTEXT_GRPC_KEYConstantThe
FULLTEXT_GRPC_KEYconstant is correctly defined to store fulltext options in gRPC column options.
260-266: Implementation ofwith_fulltext_optionsMethodThe
with_fulltext_optionsmethod correctly sets fulltext options for a column and includes appropriate error handling for serialization failures.
268-277: Implementation ofbuild_grpc_optionsMethodThe
build_grpc_optionsmethod correctly builds gRPC options for a column, ensuring that fulltext options are included if present.
279-287: Implementation ofapply_grpc_optionsMethodThe
apply_grpc_optionsmethod correctly applies gRPC options to a column schema, ensuring that fulltext options are correctly set.
Line range hint
346-353:
Definition ofFulltextOptionsStructThe
FulltextOptionsstruct is correctly defined to store fulltext options for a column.
364-371: Definition ofFulltextAnalyzerEnum andfmt::DisplayImplementationThe
FulltextAnalyzerenum is correctly defined to specify the fulltext analyzer, and thefmt::Displayimplementation is correct.src/sql/src/statements/create.rs (6)
94-98: Definition ofColumnStructThe
Columnstruct is correctly defined to include column definitions and extensions, providing a structured way to handle fulltext options.
100-103: Definition ofColumnExtensionsStructThe
ColumnExtensionsstruct is correctly defined to store fulltext options, providing an extensible way to handle additional column properties.
105-125: Implementation of Methods inColumnStructThe methods
name,data_type,mut_data_type,options, andmut_optionsare correctly implemented, providing necessary access and mutation capabilities for theColumnstruct.
127-140:DisplayImplementation forColumnStructThe
Displayimplementation for theColumnstruct correctly formats the column definition, including fulltext options if present.
86-86: Addition ofcolumnsField inCreateTableandCreateExternalTableStructsThe
columnsfield is correctly added to theCreateTableandCreateExternalTablestructs to store column definitions, including fulltext options.Also applies to: 252-252
141-141:DisplayImplementation forCreateTableandCreateExternalTableStructsThe
Displayimplementation for theCreateTableandCreateExternalTablestructs correctly formats the table definition, including columns with fulltext options.Also applies to: 257-257
src/operator/src/expr_factory.rs (5)
Line range hint
53-119:
LGTM!The function
create_external_expris correctly implemented with appropriate error handling.
Line range hint
122-155:
LGTM!The function
create_to_expris correctly implemented with appropriate error handling.
Line range hint
158-217:
LGTM!The function
validate_create_expris correctly implemented with appropriate error handling.
Line range hint
274-313:
LGTM!The function
find_primary_keysis correctly implemented with appropriate error handling.
Line range hint
356-364:
LGTM!The function
columns_to_expris correctly implemented with appropriate error handling.src/mito2/src/test_util.rs (1)
936-936: LGTM!The addition of
metadata.column_schema.build_grpc_options()ensures that the options are included in the column schema, which aligns with the PR objective of enabling full-text indexing.src/mito2/src/request.rs (1)
273-273: LGTM!The addition of
column.column_schema.build_grpc_options()ensures that the options are included in the column schema, which aligns with the PR objective of enabling full-text indexing.src/store-api/src/metadata.rs (1)
80-80: Ensureapply_grpc_optionsis correctly implemented.The introduction of
apply_grpc_optionsensures gRPC options are applied to the column schema. Verify thatapply_grpc_optionsis correctly implemented and tested.src/sql/src/statements.rs (6)
43-45: LGTM! Imports are consistent with full-text indexing functionality.The new imports
FulltextAnalyzerandFulltextOptionsare necessary for the added full-text indexing features.
57-59: LGTM! Error handling additions are appropriate.The new error variants
FulltextInvalidValueSnafuandSetFulltextOptionSnafuare necessary for handling specific cases related to full-text indexing.
61-64: LGTM! Imports are consistent with the added functionality and refactoring.The new imports
Column,OptionMap, andtransform_statementsare necessary for the added functionality and refactoring.
480-480: LGTM! Addition ofoptionsfield is consistent with new functionality.The initialization of the
optionsfield in thesql_column_def_to_grpc_column_deffunction is necessary for the new full-text indexing functionality.
Line range hint
597-607:
LGTM! Imports are necessary for testing full-text indexing functionality.The new imports
HashMapandColumnExtensionsare required for the added tests.
Line range hint
1110-1262:
LGTM! Comprehensive tests for full-text indexing functionality.The new tests cover various scenarios for the full-text indexing functionality, ensuring robust validation of the new features.
src/sql/src/parsers/create_parser.rs (6)
64-70: Validate column full-text option keys.The function
validate_column_fulltext_optioncorrectly checks if the provided key is one of the valid full-text option keys.
426-429: Ensure completeness of parse_columns function.The
parse_columnsfunction has been updated to return a tuple ofVec<Column>andVec<TableConstraint>. Ensure that all necessary constraints are parsed and returned correctly.
Line range hint
461-542:
Ensure correctness of parse_column function.The
parse_columnfunction has been updated to handle the newColumnstruct. Ensure that all column options and constraints are parsed correctly.
550-601: Ensure correctness of parse_column_def function.The
parse_column_deffunction has been updated to return aColumnstruct instead ofColumnDef. Ensure that all column attributes are parsed correctly and assigned to theColumnstruct.
655-702: Ensure correctness of parse_column_extensions function.The
parse_column_extensionsfunction correctly handles full-text options, ensuring that they are valid and unique. Ensure that all full-text options are parsed and validated correctly.
1881-1959: Ensure completeness of test_parse_create_table_fulltext_options function.The
test_parse_create_table_fulltext_optionsfunction tests various scenarios for full-text options. Ensure that all edge cases are covered in the tests.src/api/src/helper.rs (1)
1846-1846: LGTM! Ensures consistency with default options.The addition of
options: Default::default()to theColumnstruct maintains consistency and correctness.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4310 +/- ##
==========================================
- Coverage 85.13% 84.88% -0.26%
==========================================
Files 1061 1061
Lines 187834 188255 +421
==========================================
- Hits 159911 159792 -119
- Misses 27923 28463 +540 |
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (27)
- src/api/Cargo.toml (1 hunks)
- src/api/src/error.rs (1 hunks)
- src/api/src/helper.rs (1 hunks)
- src/api/src/lib.rs (1 hunks)
- src/api/src/v1/column_def.rs (2 hunks)
- src/common/grpc-expr/src/insert.rs (1 hunks)
- src/common/grpc-expr/src/util.rs (7 hunks)
- src/common/meta/src/ddl/test_util/columns.rs (1 hunks)
- src/datatypes/src/schema/column_schema.rs (4 hunks)
- src/flow/src/adapter/util.rs (2 hunks)
- src/metric-engine/src/engine/put.rs (1 hunks)
- src/metric-engine/src/test_util.rs (2 hunks)
- src/mito2/src/request.rs (2 hunks)
- src/mito2/src/test_util.rs (2 hunks)
- src/operator/src/expr_factory.rs (8 hunks)
- src/operator/src/insert.rs (1 hunks)
- src/operator/src/req_convert/common.rs (3 hunks)
- src/operator/src/req_convert/insert/stmt_to_region.rs (2 hunks)
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (3 hunks)
- src/pipeline/src/manager/table.rs (5 hunks)
- src/pipeline/tests/gsub.rs (1 hunks)
- src/pipeline/tests/join.rs (1 hunks)
- src/pipeline/tests/on_failure.rs (4 hunks)
- src/servers/src/prom_row_builder.rs (2 hunks)
- src/sql/src/error.rs (2 hunks)
- src/sql/src/statements.rs (12 hunks)
- src/store-api/src/metadata.rs (5 hunks)
Files skipped from review due to trivial changes (2)
- src/api/Cargo.toml
- src/api/src/lib.rs
Files skipped from review as they are similar to previous changes (17)
- src/api/src/v1/column_def.rs
- src/common/grpc-expr/src/insert.rs
- src/common/grpc-expr/src/util.rs
- src/common/meta/src/ddl/test_util/columns.rs
- src/datatypes/src/schema/column_schema.rs
- src/flow/src/adapter/util.rs
- src/mito2/src/request.rs
- src/operator/src/expr_factory.rs
- src/operator/src/insert.rs
- src/operator/src/req_convert/common.rs
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs
- src/pipeline/src/manager/table.rs
- src/pipeline/tests/gsub.rs
- src/pipeline/tests/join.rs
- src/pipeline/tests/on_failure.rs
- src/servers/src/prom_row_builder.rs
- src/sql/src/error.rs
Additional comments not posted (23)
src/api/src/error.rs (1)
61-68: New error variantSerializeJsonlooks good.The
SerializeJsonerror variant is correctly added to handle JSON serialization errors. Thestatus_codemethod is updated accordingly.src/operator/src/req_convert/insert/stmt_to_region.rs (2)
16-16: New importoptions_from_column_schemalooks good.The import is necessary for the new functionality and is used correctly to populate the
optionsfield inGrpcColumnSchema.
120-120: Addition ofoptionsfield inGrpcColumnSchemalooks good.The
optionsfield is correctly added to theGrpcColumnSchemastruct and is populated usingoptions_from_column_schema.src/metric-engine/src/test_util.rs (1)
248-248: Addition ofoptionsfield inPbColumnSchemalooks good.The
optionsfield is correctly added to thePbColumnSchemastruct and is set toNonefor default values.Also applies to: 255-255, 264-264
src/metric-engine/src/engine/put.rs (2)
161-161: Addition ofoptionsfield inColumnSchemalooks good.The
optionsfield is correctly added to theColumnSchemastruct and is set toNonefor default values.Also applies to: 169-169
169-169: Updates tofill_internal_columnsfunction look good.The function correctly handles the new
optionsfield, ensuring that internal columns are filled appropriately.src/mito2/src/test_util.rs (2)
32-32: Import looks good!The import
options_from_column_schemais necessary for the new functioncolumn_metadata_to_column_schema.
937-939: New functioncolumn_metadata_to_column_schemalooks good!This function correctly converts
ColumnMetadatatoapi::v1::ColumnSchemausing the newoptions_from_column_schema.src/store-api/src/metadata.rs (5)
24-24: Import looks good!The import
try_as_column_schemais necessary for the functiontry_from_column_def.
26-26: Import looks good!The import
SemanticTypeis necessary for theColumnMetadatastruct and its methods.
31-31: Import looks good!The import
ColumnSchemais necessary for theColumnMetadatastruct and its methods.
73-75: Function update looks good!The update to use
try_as_column_schemain the functiontry_from_column_defensures correct conversion ofRegionColumnDeftoColumnMetadata.
734-740: New error variantConvertColumnSchemalooks good!This error variant correctly handles errors related to the conversion of column schemas.
src/sql/src/statements.rs (8)
43-45: Imports look good!The new imports for
FulltextAnalyzer,FulltextOptions,COMMENT_KEY, and others are necessary for the added functionality related to full-text indexing.
57-59: Error handling updates are appropriate.The new error variants
FulltextInvalidOptionSnafuandSetFulltextOptionSnafuare necessary for handling errors related to full-text options.
61-64: New exports are appropriate.The new exports for
Column,OptionMap, andtransform_statementsare necessary for supporting the new functionality related to full-text indexing.
481-481: Addition of theoptionsfield insql_column_def_to_grpc_column_defis appropriate.The new
optionsfield is necessary for passing metadata and configuration through gRPC.
Line range hint
1119-1270:
Test functiontest_column_to_schemaupdates are appropriate.The added test cases ensure that full-text options are correctly handled and applied.
Line range hint
1182-1237:
New test functiontest_column_to_schema_timestamp_with_timezoneis appropriate.The new test function ensures that timestamp columns with timezones are correctly handled.
1238-1270: New test functiontest_column_to_schema_with_fulltextis appropriate.The new test function ensures that columns with full-text options are correctly handled.
Line range hint
361-430:
Review the handling of full-text options incolumn_to_schema.The function has been updated to handle full-text options. The logic appears correct, but ensure the following:
- The
fulltext_optionsare correctly extracted and applied.- Error handling for invalid options is robust.
- The function integrates well with other parts of the codebase.
Verification successful
Review the handling of full-text options in
column_to_schema.The function
column_to_schemacorrectly handles full-text options and integrates well with other parts of the codebase. The presence of thetest_column_to_schema_with_fulltexttest function ensures that the full-text options are adequately tested.
src/sql/src/statements.rs:
test_column_to_schema_with_fulltexttests the handling of full-text options.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `column_to_schema` to ensure it integrates well with other parts of the codebase. # Test: Search for the function usage. Expect: Correct handling of full-text options. rg --type rust $'column_to_schema'Length of output: 1166
Script:
#!/bin/bash # Extract the test functions related to column_to_schema to review their handling of full-text options # Get the content of test functions from statements.rs rg --type rust -A 20 'fn test_column_to_schema' src/sql/src/statements.rs # Get the content of the usage in expr_factory.rs rg --type rust -A 10 'column_to_schema' src/operator/src/expr_factory.rsLength of output: 3451
src/api/src/helper.rs (2)
1846-1846: Addition ofoptionsfield inColumnstruct.The
optionsfield has been added to theColumnstruct. Ensure that this field is correctly utilized in all relevant functions and scenarios.
1846-1848: Update inis_column_type_value_eqfunction.The function has been updated to include the
optionsfield. Ensure that the logic is consistent and correctly handles theoptions.
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/sql/src/parsers/create_parser.rs (23 hunks)
- tests/cases/standalone/common/create/create_with_fulltext.result (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/sql/src/parsers/create_parser.rs
- tests/cases/standalone/common/create/create_with_fulltext.result
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/api/src/v1/column_def.rs (2 hunks)
- src/common/grpc-expr/src/error.rs (3 hunks)
- src/datatypes/src/schema/column_schema.rs (4 hunks)
- src/sql/src/statements.rs (12 hunks)
- src/sql/src/statements/create.rs (3 hunks)
Files skipped from review as they are similar to previous changes (5)
- src/api/src/v1/column_def.rs
- src/common/grpc-expr/src/error.rs
- src/datatypes/src/schema/column_schema.rs
- src/sql/src/statements.rs
- src/sql/src/statements/create.rs
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/api/src/v1/column_def.rs (2 hunks)
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/api/src/v1/column_def.rs
- src/pipeline/src/etl/transform/transformer/greptime/coerce.rs
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
#4246
depends on GreptimeTeam/greptime-proto#179
What's changed and what's your intention?
FULLTEXTsyntaxChecklist
Summary by CodeRabbit
New Features
CreateTableandCreateExternalTablestructures with a newColumnstruct to encapsulate column properties.Bug Fixes
Documentation
Refactor
ColumnSchema.