Skip to content

Conversation

@tonyd3
Copy link

@tonyd3 tonyd3 commented Aug 5, 2025

Query Engine Cleanup and Simplification

This pull request focuses on extensive code cleanup and simplification within the query evaluation engine. It removes redundant checks, consolidates iterator creation logic, and refines memory management across several core files. The changes aim to streamline the codebase, improve readability, and potentially offer minor performance gains by reducing unnecessary overhead.

Key Changes

• Consolidated iterator creation logic in NewUnionIterator and other factory functions, removing redundant checks for zero or single child iterators.
• Removed unused functions and dead code, including RangeNumber_Free, geoRangeLoad, checkResult, and populateRange.
• Renamed ContainsCtx to TrieCallbackCtx for improved clarity and consistency in callback contexts.
• Enhanced memory management in Redis_OpenReader by ensuring RSQueryTerm is freed regardless of iterator creation success.
• Refined the quickExit logic in UnionIterator to also apply when a query node's weight is zero, optimizing result collection for non-scoring sub-trees.
• Simplified error handling and control flow in various query evaluation functions, replacing goto statements with direct returns or RS_ASSERT where appropriate.

Affected Areas

• src/query.c (Query evaluation logic)
• src/geo_index.c (Geospatial indexing and filtering)
• src/redisearch_api.c (C API for RediSearch)
• src/profile.c (Query profiling output)
• src/iterators/union_iterator.c (Union iterator implementation)
• src/redis_index.c (Redis-backed index operations)
• src/query_internal.h (Internal query declarations)


This summary was automatically generated by @propel-code-bot

@propel-code-bot propel-code-bot bot changed the title Guyav tidy ups Refactor: Simplify query evaluation and iterator handling Aug 5, 2025
Comment on lines +736 to 737
static void rangeItersAddIterator(TrieCallbackCtx *ctx, QueryIterator *it) {
ctx->its[ctx->nits++] = it;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CriticalError]

Potential buffer overrun: the element is written before ensuring that the array has enough capacity. If ctx->nits == ctx->cap, the write on the previous line indexes one element past the allocated buffer. Re-allocate first, then assign:

Suggested change
static void rangeItersAddIterator(TrieCallbackCtx *ctx, QueryIterator *it) {
ctx->its[ctx->nits++] = it;
if (ctx->nits == ctx->cap) {
ctx->cap *= 2;
ctx->its = rm_realloc(ctx->its, ctx->cap * sizeof(*ctx->its));
}
ctx->its[ctx->nits++] = it;

Committable suggestion

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Copy Context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants