Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 4926c1d

Browse files
BagritsevichStepanBagritsevichStepan
committedFeb 9, 2025··
refactor: address comments
Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
1 parent 2873f5d commit 4926c1d

File tree

1 file changed

+96
-73
lines changed

1 file changed

+96
-73
lines changed
 

‎src/server/search/search_family.cc

+96-73
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,76 @@ ParseResult<search::SchemaField::TagParams> ParseTagParams(CmdArgParser* parser)
128128
#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
129129
#endif
130130

131-
ParseResult<search::Schema> ParseSchema(DocIndex::DataType type, CmdArgParser* parser) {
132-
search::Schema schema;
131+
using ParsedSchemaField =
132+
ParseResult<std::pair<search::SchemaField::FieldType, search::SchemaField::ParamsVariant>>;
133+
134+
// Tag fields include: [separator char] [casesensitive]
135+
ParsedSchemaField ParseTag(CmdArgParser* parser) {
136+
auto tag_params = ParseTagParams(parser);
137+
if (!tag_params) {
138+
return make_unexpected(tag_params.error());
139+
}
140+
return std::make_pair(search::SchemaField::TAG, std::move(tag_params).value());
141+
}
142+
143+
ParsedSchemaField ParseText(CmdArgParser* parser) {
144+
return std::make_pair(search::SchemaField::TEXT, std::monostate{});
145+
}
146+
147+
ParsedSchemaField ParseNumeric(CmdArgParser* parser) {
148+
return std::make_pair(search::SchemaField::NUMERIC, std::monostate{});
149+
}
150+
151+
// Vector fields include: {algorithm} num_args args...
152+
ParsedSchemaField ParseVector(CmdArgParser* parser) {
153+
auto vector_params = ParseVectorParams(parser);
154+
155+
if (parser->HasError()) {
156+
auto err = *parser->Error();
157+
VLOG(1) << "Could not parse vector param " << err.index;
158+
return CreateSyntaxError("Parse error of vector parameters"sv);
159+
}
160+
161+
if (vector_params.dim == 0) {
162+
return CreateSyntaxError("Knn vector dimension cannot be zero"sv);
163+
}
164+
return std::make_pair(search::SchemaField::VECTOR, vector_params);
165+
}
166+
167+
// ON HASH | JSON
168+
ParseResult<bool> ParseOnOption(CmdArgParser* parser, DocIndex* index) {
169+
index->type = parser->MapNext("HASH"sv, DocIndex::HASH, "JSON"sv, DocIndex::JSON);
170+
return true;
171+
}
172+
173+
// PREFIX count prefix [prefix ...]
174+
ParseResult<bool> ParsePrefix(CmdArgParser* parser, DocIndex* index) {
175+
if (!parser->Check("1")) {
176+
return CreateSyntaxError("Multiple prefixes are not supported"sv);
177+
}
178+
index->prefix = parser->Next<std::string>();
179+
return true;
180+
}
181+
182+
// STOPWORDS count [words...]
183+
ParseResult<bool> ParseStopwords(CmdArgParser* parser, DocIndex* index) {
184+
index->options.stopwords.clear();
185+
for (size_t num = parser->Next<size_t>(); num > 0; num--) {
186+
index->options.stopwords.emplace(parser->Next());
187+
}
188+
return true;
189+
}
190+
191+
// SCHEMA field [AS alias] type [flags...]
192+
ParseResult<bool> ParseSchema(CmdArgParser* parser, DocIndex* index) {
193+
auto& schema = index->schema;
133194

134195
while (parser->HasNext()) {
135196
string_view field = parser->Next();
136197
string_view field_alias = field;
137198

138199
// Verify json path is correct
139-
if (type == DocIndex::JSON && !IsValidJsonPath(field)) {
200+
if (index->type == DocIndex::JSON && !IsValidJsonPath(field)) {
140201
return CreateSyntaxError(absl::StrCat("Bad json path: "sv, field));
141202
}
142203

@@ -145,61 +206,38 @@ ParseResult<search::Schema> ParseSchema(DocIndex::DataType type, CmdArgParser* p
145206

146207
// Determine type
147208
using search::SchemaField;
148-
auto type = parser->MapNext("TAG"sv, SchemaField::TAG, "TEXT"sv, SchemaField::TEXT, "NUMERIC"sv,
149-
SchemaField::NUMERIC, "VECTOR"sv, SchemaField::VECTOR);
150-
151-
// Tag fields include: [separator char] [casesensitive]
152-
// Vector fields include: {algorithm} num_args args...
153-
search::SchemaField::ParamsVariant params(monostate{});
154-
if (type == search::SchemaField::TAG) {
155-
auto tag_params = ParseTagParams(parser);
156-
if (!tag_params) {
157-
return make_unexpected(tag_params.error());
158-
}
159-
params = tag_params.value();
160-
} else if (type == search::SchemaField::VECTOR && !parser->HasError()) {
161-
auto vector_params = ParseVectorParams(parser);
162-
163-
if (parser->HasError()) {
164-
auto err = *parser->Error();
165-
VLOG(1) << "Could not parse vector param " << err.index;
166-
return CreateSyntaxError("Parse error of vector parameters"sv);
167-
}
168-
169-
if (vector_params.dim == 0) {
170-
return CreateSyntaxError("Knn vector dimension cannot be zero"sv);
171-
}
172-
params = vector_params;
209+
auto parsed_params = parser->MapNext("TAG"sv, &ParseTag, "TEXT"sv, &ParseText, "NUMERIC"sv,
210+
&ParseNumeric, "VECTOR"sv, &ParseVector)(parser);
211+
if (!parsed_params) {
212+
return make_unexpected(parsed_params.error());
173213
}
174214

215+
auto [field_type, params] = std::move(parsed_params).value();
216+
175217
// Flags: check for SORTABLE and NOINDEX
176218
uint8_t flags = 0;
177219
while (parser->HasNext()) {
178-
if (parser->Check("NOINDEX")) {
179-
flags |= search::SchemaField::NOINDEX;
180-
continue;
220+
auto flag = parser->TryMapNext("NOINDEX", search::SchemaField::NOINDEX, "SORTABLE",
221+
search::SchemaField::SORTABLE);
222+
if (!flag) {
223+
break;
181224
}
182225

183-
if (parser->Check("SORTABLE")) {
184-
flags |= search::SchemaField::SORTABLE;
185-
continue;
186-
}
187-
188-
break;
226+
flags |= *flag;
189227
}
190228

191229
// Skip all trailing ignored parameters
192230
while (kIgnoredOptions.count(parser->Peek()) > 0)
193231
parser->Skip(2);
194232

195-
schema.fields[field] = {type, flags, string{field_alias}, params};
233+
schema.fields[field] = {field_type, flags, string{field_alias}, params};
196234
}
197235

198236
// Build field name mapping table
199237
for (const auto& [field_ident, field_info] : schema.fields)
200238
schema.field_names[field_info.short_name] = field_ident;
201239

202-
return schema;
240+
return false;
203241
}
204242

205243
#ifndef __clang__
@@ -210,39 +248,23 @@ ParseResult<DocIndex> ParseCreateParams(CmdArgParser* parser) {
210248
DocIndex index{};
211249

212250
while (parser->HasNext()) {
213-
// ON HASH | JSON
214-
if (parser->Check("ON")) {
215-
index.type = parser->MapNext("HASH"sv, DocIndex::HASH, "JSON"sv, DocIndex::JSON);
216-
continue;
217-
}
251+
auto option_parser =
252+
parser->TryMapNext("ON"sv, &ParseOnOption, "PREFIX"sv, &ParsePrefix, "STOPWORDS"sv,
253+
&ParseStopwords, "SCHEMA"sv, &ParseSchema);
218254

219-
// PREFIX count prefix [prefix ...]
220-
if (parser->Check("PREFIX")) {
221-
if (!parser->Check("1"))
222-
return CreateSyntaxError("Multiple prefixes are not supported"sv);
223-
index.prefix = string(parser->Next());
255+
if (!option_parser) {
256+
// Unsupported parameters are ignored for now
257+
parser->Skip(1);
224258
continue;
225259
}
226260

227-
// STOWORDS count [words...]
228-
if (parser->Check("STOPWORDS")) {
229-
index.options.stopwords.clear();
230-
for (size_t num = parser->Next<size_t>(); num > 0; num--)
231-
index.options.stopwords.emplace(parser->Next());
232-
continue;
261+
auto parse_result = option_parser.value()(parser, &index);
262+
if (!parse_result) {
263+
return make_unexpected(parse_result.error());
233264
}
234-
235-
// SCHEMA
236-
if (parser->Check("SCHEMA")) {
237-
auto schema = ParseSchema(index.type, parser);
238-
if (!schema)
239-
return make_unexpected(schema.error());
240-
index.schema = std::move(*schema);
241-
break; // SCHEMA always comes last
265+
if (!parse_result.value()) {
266+
break;
242267
}
243-
244-
// Unsupported parameters are ignored for now
245-
parser->Skip(1);
246268
}
247269

248270
return index;
@@ -661,22 +683,23 @@ void SearchFamily::FtAlter(CmdArgList args, const CommandContext& cmd_cntx) {
661683
}
662684

663685
// Parse additional schema
664-
auto new_fields = ParseSchema(index_info->type, &parser);
665-
if (SendErrorIfOccurred(new_fields, &parser, builder)) {
686+
DocIndex new_index{.type = index_info->type};
687+
auto parse_result = ParseSchema(&parser, &new_index);
688+
if (SendErrorIfOccurred(parse_result, &parser, builder)) {
666689
cmd_cntx.tx->Conclude();
667690
return;
668691
}
669692

693+
auto& new_fields = new_index.schema;
694+
670695
// For logging we copy the whole schema
671696
// TODO: Use a more efficient way for logging
672-
LOG(INFO)
673-
<< "Adding "
674-
<< DocIndexInfo{.base_index = DocIndex{.schema = new_fields.value()}}.BuildRestoreCommand();
697+
LOG(INFO) << "Adding " << DocIndexInfo{.base_index = new_index}.BuildRestoreCommand();
675698

676699
// Merge schemas
677700
search::Schema& schema = index_info->schema;
678-
schema.fields.insert(new_fields->fields.begin(), new_fields->fields.end());
679-
schema.field_names.insert(new_fields->field_names.begin(), new_fields->field_names.end());
701+
schema.fields.insert(new_fields.fields.begin(), new_fields.fields.end());
702+
schema.field_names.insert(new_fields.field_names.begin(), new_fields.field_names.end());
680703

681704
// Rebuild index
682705
// TODO: Introduce partial rebuild

0 commit comments

Comments
 (0)
Please sign in to comment.