From e7121933c4d137783680ae5991426d82a40551b1 Mon Sep 17 00:00:00 2001 From: Roi Lipman Date: Wed, 15 Jul 2020 11:56:37 +0300 Subject: [PATCH] Update index on change (#1225) * WIP * Fix compile errors * Only update indexes when necessary * refined update entity update eval * WIP * WIP * Compilation fixes, edge updates * Add freeing logic * Update comments * some minor changes to op_update * Address PR comments * Address PR comments * always get updated node label id Co-authored-by: Jeffrey Lovitz Co-authored-by: DvirDukhan --- src/execution_plan/execution_plan.c | 1 + src/execution_plan/ops/op_update.c | 408 ++++++++++++++++++---------- src/execution_plan/ops/op_update.h | 38 +-- src/graph/entities/node.h | 5 +- src/index/index.h | 3 +- src/schema/schema.c | 38 +-- 6 files changed, 310 insertions(+), 183 deletions(-) diff --git a/src/execution_plan/execution_plan.c b/src/execution_plan/execution_plan.c index 322f2c93ca..2d98f1d0d7 100644 --- a/src/execution_plan/execution_plan.c +++ b/src/execution_plan/execution_plan.c @@ -636,6 +636,7 @@ static inline void _buildUpdateOp(GraphContext *gc, ExecutionPlan *plan, const cypher_astnode_t *clause) { EntityUpdateEvalCtx *update_exps = AST_PrepareUpdateOp(gc, clause); OpBase *op = NewUpdateOp(plan, update_exps); + array_free(update_exps); _ExecutionPlan_UpdateRoot(plan, op); } diff --git a/src/execution_plan/ops/op_update.c b/src/execution_plan/ops/op_update.c index ca49eeeb08..70ca76a6f0 100644 --- a/src/execution_plan/ops/op_update.c +++ b/src/execution_plan/ops/op_update.c @@ -7,6 +7,7 @@ #include "op_update.h" #include "../../query_ctx.h" #include "../../util/arr.h" +#include "../../util/qsort.h" #include "../../util/rmalloc.h" #include "../../arithmetic/arithmetic_expression.h" #include "../../query_ctx.h" @@ -18,74 +19,29 @@ static OpResult UpdateReset(OpBase *opBase); static OpBase *UpdateClone(const ExecutionPlan *plan, const OpBase *opBase); static void UpdateFree(OpBase *opBase); -/* Delay updates until all entities are processed, - * _QueueUpdate will queue up all information necessary to perform an update. */ -static void _QueueUpdate(OpUpdate *op, GraphEntity *entity, GraphEntityType type, - Attribute_ID attr_id, SIValue new_value) { - /* Make sure we've got enough room in queue. */ - if(op->pending_updates_count == op->pending_updates_cap) { - op->pending_updates_cap *= 2; - op->pending_updates = rm_realloc(op->pending_updates, - op->pending_updates_cap * sizeof(EntityUpdateCtx)); - } - - uint i = op->pending_updates_count; - op->pending_updates[i].new_value = new_value; - op->pending_updates[i].attr_id = attr_id; - op->pending_updates[i].entity_type = type; - // Copy updated entity. - if(type == GETYPE_NODE) { - op->pending_updates[i].n = *((Node *)entity); - } else { - op->pending_updates[i].e = *((Edge *)entity); - } - op->pending_updates_count++; -} - -/* Introduce updated entity to index. */ -static void _UpdateIndex(EntityUpdateCtx *ctx, GraphContext *gc, Schema *s, SIValue *old_value, - SIValue *new_value) { - Node *n = &ctx->n; - - // reindex - Schema_AddNodeToIndices(s, n); -} - -/* Set a property on a node. For non-NULL values, the property - * will be added or updated if it is already present. - * For NULL values, the property will be deleted if present - * and nothing will be done otherwise. - * Relevant indexes will be updated accordingly. - * Returns 1 if a property was set or deleted. */ -static int _UpdateNode(OpUpdate *op, EntityUpdateCtx *ctx) { - /* Retrieve GraphEntity: - * Due to Record freeing we can't maintain the original pointer to GraphEntity object, - * but only a pointer to an Entity object, - * to use the GraphEntity_Get, GraphEntity_Add functions we'll use a place holder - * to hold our entity. */ - Schema *s = NULL; - Node *node = &ctx->n; - - int label_id = Graph_GetNodeLabel(op->gc->g, ENTITY_GET_ID(node)); - if(label_id != GRAPH_NO_LABEL) { - s = GraphContext_GetSchemaByID(op->gc, label_id, SCHEMA_NODE); - } +static int _UpdateEntity(GraphEntity *ge, PendingUpdateCtx *update) { + int res = 1; + SIValue new_value = update->new_value; + Attribute_ID attr_id = update->attr_id; // Try to get current property value. - SIValue *old_value = GraphEntity_GetProperty((GraphEntity *)node, ctx->attr_id); + SIValue *old_value = GraphEntity_GetProperty(ge, attr_id); if(old_value == PROPERTY_NOTFOUND) { // Adding a new property; do nothing if its value is NULL. - if(SI_TYPE(ctx->new_value) == T_NULL) return 0; - GraphEntity_AddProperty((GraphEntity *)node, ctx->attr_id, ctx->new_value); + if(SI_TYPE(new_value) == T_NULL) { + res = 0; + goto cleanup; + } + GraphEntity_AddProperty(ge, attr_id, new_value); } else { // Update property. - GraphEntity_SetProperty((GraphEntity *)node, ctx->attr_id, ctx->new_value); + GraphEntity_SetProperty(ge, attr_id, new_value); } - // Update index for node entities. - _UpdateIndex(ctx, op->gc, s, old_value, &ctx->new_value); - return 1; +cleanup: + SIValue_Free(new_value); + return res; } /* Set a property on an edge. For non-NULL values, the property @@ -93,46 +49,90 @@ static int _UpdateNode(OpUpdate *op, EntityUpdateCtx *ctx) { * For NULL values, the property will be deleted if present * and nothing will be done otherwise. * Returns 1 if a property was set or deleted. */ -static int _UpdateEdge(OpUpdate *op, EntityUpdateCtx *ctx) { +static int _UpdateEdge(OpUpdate *op, PendingUpdateCtx *updates, + uint update_count) { /* Retrieve GraphEntity: - * Due to Record freeing we can't maintain the original pointer to GraphEntity object, - * but only a pointer to an Entity object, - * to use the GraphEntity_Get, GraphEntity_Add functions we'll use a place holder - * to hold our entity. */ - Edge *edge = &ctx->e; + * Due to Record freeing we can't maintain the original pointer to + * GraphEntity object, but only a pointer to an Entity object, to use the + * GraphEntity_Get, GraphEntity_Add functions we'll use a place holder to + * hold our entity. */ + int attributes_set = 0; + GraphEntity *ge = (GraphEntity *)&updates->e; + + for(uint i = 0; i < update_count; i++) { + PendingUpdateCtx *update = updates + i; + attributes_set += _UpdateEntity(ge, update); + } - int label_id = Graph_GetEdgeRelation(op->gc->g, edge); + return attributes_set; +} - // Try to get current property value. - SIValue *old_value = GraphEntity_GetProperty((GraphEntity *)edge, ctx->attr_id); +/* Set a property on a node. For non-NULL values, the property + * will be added or updated if it is already present. + * For NULL values, the property will be deleted if present + * and nothing will be done otherwise. + * Relevant indexes will be updated if required. + * Returns 1 if a property was set or deleted. */ +static int _UpdateNode(OpUpdate *op, PendingUpdateCtx *updates, + uint update_count) { + /* Retrieve GraphEntity: + * Due to Record freeing we can't maintain the original pointer to + * GraphEntity object, but only a pointer to an Entity object, to use the + * GraphEntity_Get, GraphEntity_Add functions we'll use a place holder to + * hold our entity. */ + int attributes_set = 0; + bool update_index = false; + Node *node = &updates->n; + GraphEntity *ge = (GraphEntity *)node; + + for(uint i = 0; i < update_count; i++) { + PendingUpdateCtx *update = updates + i; + attributes_set += _UpdateEntity(ge, update); + // Do we need to update an index for this property? + update_index |= update->update_index; + } - if(old_value == PROPERTY_NOTFOUND) { - // Adding a new property; do nothing if its value is NULL. - if(SI_TYPE(ctx->new_value) == T_NULL) return 0; - GraphEntity_AddProperty((GraphEntity *)edge, ctx->attr_id, ctx->new_value); - } else { - // Update property. - GraphEntity_SetProperty((GraphEntity *)edge, ctx->attr_id, ctx->new_value); + // Update index for node entities if indexed fields have been modified. + if(update_index) { + int label_id = node->labelID; + Schema *s = GraphContext_GetSchemaByID(op->gc, label_id, SCHEMA_NODE); + // Introduce updated entity to index. + Schema_AddNodeToIndices(s, node); } - return 1; + + return attributes_set; } -/* Executes delayed updates. */ -static void _CommitUpdates(OpUpdate *op) { +static void _CommitEntityUpdates(OpUpdate *op, EntityUpdateCtx *ctx) { uint properties_set = 0; - for(uint i = 0; i < op->pending_updates_count; i++) { - EntityUpdateCtx *ctx = &op->pending_updates[i]; - if(ctx->entity_type == GETYPE_NODE) { - properties_set += _UpdateNode(op, ctx); + uint updates_per_entity = array_len(ctx->exps); + // Total_updates = updates_per_entity * number of entities being updated. + uint total_updates = array_len(ctx->updates); + + /* For each iteration of this loop, perform all updates + * enqueued for a single entity. */ + for(uint i = 0; i < total_updates; i += updates_per_entity) { + // Set a pointer to the first update context for this entity. + PendingUpdateCtx *update_ctx = ctx->updates + i; + if(update_ctx->entity_type == GETYPE_NODE) { + properties_set += _UpdateNode(op, update_ctx, updates_per_entity); } else { - properties_set += _UpdateEdge(op, ctx); + properties_set += _UpdateEdge(op, update_ctx, updates_per_entity); } - SIValue_Free(ctx->new_value); } if(op->stats) op->stats->properties_set += properties_set; } +// Commits delayed updates. +static void _CommitUpdates(OpUpdate *op) { + uint entity_count = array_len(op->update_ctxs); + for(uint i = 0; i < entity_count; i++) { + EntityUpdateCtx *entity_ctx = &op->update_ctxs[i]; + _CommitEntityUpdates(op, entity_ctx); + } +} + /* We only cache records if op_update is not the last * operation within the execution-plan, which means * others operations might inspect thoese records. @@ -149,25 +149,66 @@ static Record _handoff(OpUpdate *op) { return NULL; } -OpBase *NewUpdateOp(const ExecutionPlan *plan, EntityUpdateEvalCtx *update_exps) { +static void _groupUpdateExps(OpUpdate *op, EntityUpdateEvalCtx *update_ctxs) { + // Sort update contexts by unique Record ID. + #define islt(a,b) (a->record_idx < b->record_idx) + + uint n = array_len(update_ctxs); + QSORT(EntityUpdateEvalCtx, update_ctxs, n, islt); + + /* Create an EntityUpdateCtx for each alias that will be modified by this op. + * Group all relevant EvalCtx structs on each new context. */ + op->update_ctxs = array_new(EntityUpdateCtx, 1); + + EntityUpdateEvalCtx *prev = NULL; + EntityUpdateCtx *entity_ctx = NULL; + + for(uint i = 0; i < n; i++) { + EntityUpdateEvalCtx *current = &update_ctxs[i]; + if(!prev || current->record_idx != prev->record_idx) { + // Encountered a diffrent entity being updated; create a new context. + EntityUpdateCtx new_ctx = { .alias = current->alias, + .record_idx = current->record_idx, + .updates = array_new(PendingUpdateCtx, 1), + .exps = array_new(EntityUpdateEvalCtx, 1), + }; + + // Append the new context to the array of contexts. + op->update_ctxs = array_append(op->update_ctxs, new_ctx); + + // Update the entity_ctx pointer to reference the new context. + uint latest_group_idx = array_len(op->update_ctxs) - 1; + entity_ctx = &op->update_ctxs[latest_group_idx]; + + prev = current; + } + + // Append the current expression to the latest context. + entity_ctx->exps = array_append(entity_ctx->exps, *current); + } +} + +OpBase *NewUpdateOp(const ExecutionPlan *plan, EntityUpdateEvalCtx + *update_exps) { OpUpdate *op = rm_calloc(1, sizeof(OpUpdate)); - op->gc = QueryCtx_GetGraphCtx(); op->records = NULL; + op->update_ctxs = NULL; op->updates_commited = false; - op->pending_updates_cap = 16; /* 16 seems reasonable number to start with. */ - op->pending_updates_count = 0; - op->update_expressions = update_exps; - op->update_expressions_count = array_len(update_exps); - op->pending_updates = rm_malloc(sizeof(EntityUpdateCtx) * op->pending_updates_cap); + op->gc = QueryCtx_GetGraphCtx(); - // Set our Op operations + // Set our Op operations. OpBase_Init((OpBase *)op, OPType_UPDATE, "Update", UpdateInit, UpdateConsume, UpdateReset, NULL, UpdateClone, UpdateFree, true, plan); - for(uint i = 0; i < op->update_expressions_count; i ++) { - op->update_expressions[i].record_idx = OpBase_Modifies((OpBase *)op, update_exps[i].alias); + // Set the record index for every entity modified by this operation. + uint exp_count = array_len(update_exps); + for(uint i = 0; i < exp_count; i++) { + update_exps[i].record_idx = OpBase_Modifies((OpBase *)op, update_exps[i].alias); } + // Group update expression by entity. + _groupUpdateExps(op, update_exps); + return (OpBase *)op; } @@ -178,6 +219,83 @@ static OpResult UpdateInit(OpBase *opBase) { return OP_OK; } +static void _EvalEntityUpdates(EntityUpdateCtx *ctx, GraphContext *gc, + Record r) { + Schema *s = NULL; + const char *label = NULL; + bool update_index = false; + int label_id = GRAPH_NO_LABEL; + + // Get the type of the entity to update. If the expected entity was not + // found, make no updates but do not error. + RecordEntryType t = Record_GetType(r, ctx->record_idx); + if(t == REC_TYPE_UNKNOWN) return; + + // Make sure we're updating either a node or an edge. + if(t != REC_TYPE_NODE && t != REC_TYPE_EDGE) { + QueryCtx_SetError("Update error: alias '%s' did not resolve to a graph entity", + ctx->alias); + QueryCtx_RaiseRuntimeException(); + } + + GraphEntity *entity = Record_GetGraphEntity(r, ctx->record_idx); + GraphEntityType type = (t == REC_TYPE_NODE) ? GETYPE_NODE : GETYPE_EDGE; + + // If the entity is a node, set its label if possible. + if(type == GETYPE_NODE) { + Node *n = (Node *)entity; + label_id = Graph_GetNodeLabel(gc->g, ENTITY_GET_ID(entity)); + + if(label_id != GRAPH_NO_LABEL) { + s = GraphContext_GetSchemaByID(gc, label_id, SCHEMA_NODE); + label = Schema_GetName(s); + } + + n->label = label; + n->labelID = label_id; + } + + uint exp_count = array_len(ctx->exps); + for(uint i = 0; i < exp_count; i++) { + EntityUpdateEvalCtx *update_ctx = ctx->exps + i; + SIValue new_value = AR_EXP_Evaluate(update_ctx->exp, r); + + /* Determine whether we must update the index for this set of updates. + * If at least one property being updated is indexed, each node will be reindexed. */ + if(!update_index && label) { + Attribute_ID attr_id = update_ctx->attribute_id; + const char *field = GraphContext_GetAttributeString(gc, attr_id); + // If the label-index combination has an index, we must reindex this entity. + update_index = GraphContext_GetIndex(gc, label, field, IDX_ANY) != NULL; + if(update_index && (i > 0)) { + /* Swap the current update expression with the first one + * so that subsequent searches will find the index immediately. */ + EntityUpdateEvalCtx first = ctx->exps[0]; + ctx->exps[0] = ctx->exps[i]; + ctx->exps[i] = first; + } + } + + PendingUpdateCtx update = { + .entity_type = type, + .new_value = new_value, + .update_index = update_index, + .attr_id = update_ctx->attribute_id, + }; + + if(type == GETYPE_EDGE) { + // Add the edge to the update context. + update.e = *((Edge *)entity); + } else { + // Add the node to the update context. + update.n = *((Node *)entity); + } + + // Enqueue the current update. + ctx->updates = array_append(ctx->updates, update); + } +} + static Record UpdateConsume(OpBase *opBase) { OpUpdate *op = (OpUpdate *)opBase; OpBase *child = op->op.children[0]; @@ -186,26 +304,12 @@ static Record UpdateConsume(OpBase *opBase) { // Updates already performed. if(op->updates_commited) return _handoff(op); - while((r = OpBase_Consume(child))) { - /* Evaluate each update expression and store result - * for later execution. */ - EntityUpdateEvalCtx *update_expression = op->update_expressions; - for(uint i = 0; i < op->update_expressions_count; i++, update_expression++) { - // Get the type of the entity to update. - RecordEntryType t = Record_GetType(r, update_expression->record_idx); - // If the expected entity was not found, make no updates but do not error. - if(t == REC_TYPE_UNKNOWN) continue; - // Make sure we're updating either a node or an edge. - if(t != REC_TYPE_NODE && t != REC_TYPE_EDGE) { - QueryCtx_SetError("Update error: alias '%s' did not resolve to a graph entity", - update_expression->alias); - QueryCtx_RaiseRuntimeException(); - } - GraphEntityType type = (t == REC_TYPE_NODE) ? GETYPE_NODE : GETYPE_EDGE; - GraphEntity *entity = Record_GetGraphEntity(r, update_expression->record_idx); + uint nctx = array_len(op->update_ctxs); - SIValue new_value = SI_CloneValue(AR_EXP_Evaluate(update_expression->exp, r)); - _QueueUpdate(op, entity, type, update_expression->attribute_id, new_value); + while((r = OpBase_Consume(child))) { + // Evaluate update expressions. + for(uint i = 0; i < nctx; i++) { + _EvalEntityUpdates(op->update_ctxs + i, op->gc, r); } if(_ShouldCacheRecord(op)) { @@ -216,12 +320,12 @@ static Record UpdateConsume(OpBase *opBase) { } } - /* Done reading, we're not going to call consume any longer - * there might be operations e.g. index scan that need to free - * index R/W lock, as such free all execution plan operation up the chain. */ + /* Done reading; we're not going to call Consume any longer. + * There might be operations like Index Scan that need to free the + * index R/W lock - as such, free all ExecutionPlan operations up the chain. */ OpBase_PropagateFree(child); - /* Lock everything. */ + // Lock everything. QueryCtx_LockForCommit(); _CommitUpdates(op); // Release lock. @@ -231,32 +335,64 @@ static Record UpdateConsume(OpBase *opBase) { return _handoff(op); } +// Collect all EvalCtx structs held within elements of the inputs array. +static EntityUpdateEvalCtx *_CollectEvalContexts(EntityUpdateCtx *inputs) { + EntityUpdateEvalCtx *eval_contexts = array_new(EntityUpdateEvalCtx, 1); + uint nctx = array_len(inputs); + + // For each update context, collect all EvalCtx structs. + for(uint i = 0; i < nctx; i++) { + EntityUpdateCtx update_ctx = inputs[i]; + uint nexps = array_len(update_ctx.exps); + + for(uint j = 0; j < nexps; j++) { + EntityUpdateEvalCtx exp = update_ctx.exps[j]; + eval_contexts = array_append(eval_contexts, EntityUpdateEvalCtx_Clone(exp)); + } + } + + return eval_contexts; +} + static OpBase *UpdateClone(const ExecutionPlan *plan, const OpBase *opBase) { assert(opBase->type == OPType_UPDATE); OpUpdate *op = (OpUpdate *)opBase; - EntityUpdateEvalCtx *update_exps; - array_clone_with_cb(update_exps, op->update_expressions, EntityUpdateEvalCtx_Clone); - return NewUpdateOp(plan, update_exps); + + // Recreate original input. + EntityUpdateEvalCtx *update_exps = _CollectEvalContexts(op->update_ctxs); + OpBase *clone = NewUpdateOp(plan, update_exps); + array_free(update_exps); + + return clone; } static OpResult UpdateReset(OpBase *ctx) { OpUpdate *op = (OpUpdate *)ctx; - // Reset all pending updates. - op->pending_updates_count = 0; - op->pending_updates_cap = 16; /* 16 seems reasonable number to start with. */ - op->pending_updates = rm_realloc(op->pending_updates, - op->pending_updates_cap * sizeof(EntityUpdateCtx)); return OP_OK; } +static void _UpdateContexts_Free(EntityUpdateCtx *contexts) { + uint ctx_count = array_len(contexts); + for(uint i = 0; i < ctx_count; i++) { + EntityUpdateCtx update_ctx = contexts[i]; + if(update_ctx.exps) { + uint eval_ctx_count = array_len(update_ctx.exps); + for(uint j = 0; j < eval_ctx_count; j++) { + AR_EXP_Free(update_ctx.exps[j].exp); + } + array_free(update_ctx.exps); + } + if(update_ctx.updates) array_free(update_ctx.updates); + } +} + static void UpdateFree(OpBase *ctx) { OpUpdate *op = (OpUpdate *)ctx; - /* Free each update context. */ - if(op->update_expressions_count) { - for(uint i = 0; i < op->update_expressions_count; i++) { - AR_EXP_Free(op->update_expressions[i].exp); - } - op->update_expressions_count = 0; + // Free each update context. + if(op->update_ctxs) { + _UpdateContexts_Free(op->update_ctxs); + array_free(op->update_ctxs); + op->update_ctxs = NULL; } if(op->records) { @@ -265,15 +401,5 @@ static void UpdateFree(OpBase *ctx) { array_free(op->records); op->records = NULL; } - - if(op->update_expressions) { - array_free(op->update_expressions); - op->update_expressions = NULL; - } - - if(op->pending_updates) { - rm_free(op->pending_updates); - op->pending_updates = NULL; - } } diff --git a/src/execution_plan/ops/op_update.h b/src/execution_plan/ops/op_update.h index 9ca94afaec..02430a879d 100644 --- a/src/execution_plan/ops/op_update.h +++ b/src/execution_plan/ops/op_update.h @@ -14,15 +14,23 @@ #include "../../arithmetic/arithmetic_expression.h" #include "../../ast/ast_build_op_contexts.h" -// Context describing a pending update to perform. +// Context grouping a set of updates to perform on a single entity. typedef struct { - Attribute_ID attr_id; /* ID of attribute to update. */ union { - Node n; /* Node to update if indicated by entity_type. */ - Edge e; /* Edge to update if indicated by entity_type. */ - }; - GraphEntityType entity_type; /* Graph entity type. */ - SIValue new_value; /* Constant value to set. */ + Node n; + Edge e; + }; // Updated entity. + GraphEntityType entity_type; // Type of updated entity. + bool update_index; // Does index effected by update. + SIValue new_value; // Constant value to set. + Attribute_ID attr_id; // Id of attribute to update. +} PendingUpdateCtx; + +typedef struct { + int record_idx; // Record offset this entity is stored at. + const char *alias; // Updated entity alias. + EntityUpdateEvalCtx *exps; // Update expressions converted from the AST. + PendingUpdateCtx *updates; // List of pending updates for this op to commit. } EntityUpdateCtx; typedef struct { @@ -30,17 +38,11 @@ typedef struct { GraphContext *gc; ResultSetStatistics *stats; - uint update_expressions_count; - EntityUpdateEvalCtx - *update_expressions; /* List of entities to update and their arithmetic expressions. */ - - uint pending_updates_cap; - uint pending_updates_count; - EntityUpdateCtx - *pending_updates; /* List of entities to update and their actual new value. */ - Record *records; /* Updated records, used only when query inspects updated entities. */ - bool updates_commited; /* Updates performed? */ + EntityUpdateCtx *update_ctxs; // Entities to update and their expressions. + Record *records; // Updated records, used only when query hands off records after updates. + bool updates_commited; // True if we've already committed updates and are now in handoff mode. } OpUpdate; -OpBase *NewUpdateOp(const ExecutionPlan *plan, EntityUpdateEvalCtx *update_exps); +OpBase *NewUpdateOp(const ExecutionPlan *plan, + EntityUpdateEvalCtx *update_exps); diff --git a/src/graph/entities/node.h b/src/graph/entities/node.h index 1bd76fadb7..b964335082 100644 --- a/src/graph/entities/node.h +++ b/src/graph/entities/node.h @@ -21,9 +21,6 @@ typedef struct { /* Creates a new node. */ Node *Node_New(const char *label); -/* Sets node relation type. */ -void Node_SetLabelID(Node *n, int labelID); // QG only - /* Retrieves node matrix. */ GrB_Matrix Node_GetMatrix(Node *n); // AE @@ -37,4 +34,4 @@ void Node_ToString(const Node *n, char **buffer, size_t *bufferLen, size_t *byte /* Frees allocated space by given node. */ void Node_Free(Node *node); -#endif \ No newline at end of file +#endif diff --git a/src/index/index.h b/src/index/index.h index 6dc7ad6901..45308f434d 100644 --- a/src/index/index.h +++ b/src/index/index.h @@ -14,6 +14,7 @@ #define INDEX_FAIL 0 typedef enum { + IDX_ANY, IDX_EXACT_MATCH, IDX_FULLTEXT, } IndexType; @@ -105,4 +106,4 @@ bool Index_ContainsField void Index_Free ( Index *idx -); \ No newline at end of file +); diff --git a/src/schema/schema.c b/src/schema/schema.c index 3833ad771f..666f1fc1c5 100644 --- a/src/schema/schema.c +++ b/src/schema/schema.c @@ -43,8 +43,14 @@ unsigned short Schema_IndexCount(const Schema *s) { Index *Schema_GetIndex(const Schema *s, const char *field, IndexType type) { Index *idx = NULL; - if(type == IDX_EXACT_MATCH) idx = s->index; - else idx = s->fulltextIdx; + if(type == IDX_EXACT_MATCH) { + idx = s->index; + } else if(type == IDX_FULLTEXT) { + idx = s->fulltextIdx; + } else { + // If type is unspecified, use the first index that exists. + idx = s->index ? : s->fulltextIdx; + } if(!idx) return NULL; @@ -84,28 +90,22 @@ int Schema_RemoveIndex(Schema *s, const char *field, IndexType type) { Index *idx = Schema_GetIndex(s, field, type); if(idx == NULL) return INDEX_FAIL; - /* Currently dropping a full-text index - * doesn't take into account fields. */ + type = idx->type; + + // Currently dropping a full-text index doesn't take into account fields. if(type == IDX_FULLTEXT) { assert(field == NULL); Index_Free(idx); s->fulltextIdx = NULL; - return INDEX_OK; - } - - Index_RemoveField(idx, field); - - /* If index field count dropped to 0 - * remove index from schema. */ - if(Index_FieldsCount(idx) == 0) { - Index_Free(idx); - switch(type) { - case IDX_EXACT_MATCH: + } else { + // Index is of type IDX_EXACT_MATCH + assert(type == IDX_EXACT_MATCH); + Index_RemoveField(idx, field); + + // If index field count dropped to 0, remove index from schema. + if(Index_FieldsCount(idx) == 0) { + Index_Free(idx); s->index = NULL; - break; - case IDX_FULLTEXT: - s->fulltextIdx = NULL; - break; } }