Skip to content

feat: add SQLQuery support to AsyncSearchIndex (#487)#521

Merged
nkanu17 merged 1 commit intomainfrom
feature/async-sql-query
Mar 2, 2026
Merged

feat: add SQLQuery support to AsyncSearchIndex (#487)#521
nkanu17 merged 1 commit intomainfrom
feature/async-sql-query

Conversation

@nkanu17
Copy link
Collaborator

@nkanu17 nkanu17 commented Mar 2, 2026

fixes #487

  • Add async SQL query execution using sql-redis AsyncSchemaRegistry and AsyncExecutor
  • Update sql-redis dependency to >=0.2.0
  • Add integration tests for async SQL SELECT and AGGREGATE queries

Validation:
Validate that the async SQL query implementation in RedisVL provides real concurrency benefits, not just an async wrapper around blocking calls.

Validation experiment

  • Documents: 1,000 loaded into Redis
  • Queries per test: 50 (mix of SELECT, COUNT, GROUP BY, ORDER BY)
  • Test scenarios:
    1. Sync sequential — SearchIndex.query() in a loop
    2. Async sequential — await AsyncSearchIndex.query() in a loop
    3. Async concurrent — asyncio.gather() with multiple queries

Results

Approach Time vs Sync
Sync Sequential 0.35s
Async Sequential 0.16s 2.1x faster
Async Concurrent 0.10s 3.7x faster

Outcome

  1. Async sequential is 2x faster than sync, proving it's not just a wrapper around blocking calls

  2. Using asyncio.gather() provides 1.7x speedup over sequential async execution

  3. Redis is single-threaded, so server-side query execution is sequential; the benefit comes from overlapping client-side I/O wait time


Note

Medium Risk
Adds a new async SQL execution path in AsyncSearchIndex.query() and bumps the sql-redis dependency, which could affect query behavior/decoding and introduce new runtime ImportError paths when the extra isn’t installed.

Overview
AsyncSearchIndex.query() now accepts SQLQuery and executes it via sql-redis’s AsyncSchemaRegistry/AsyncExecutor, returning rows after convert_bytes normalization (mirroring the sync SQL path).

The sql-redis optional extra is bumped to >=0.2.0, and integration coverage is expanded with async SQL SELECT and COUNT(*) cases.

Docs are updated with an Async Support example in 12_sql_to_redis_queries.ipynb demonstrating running SQLQuery against AsyncSearchIndex.

Written by Cursor Bugbot for commit 8a15f49. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings March 2, 2026 18:38
@jit-ci
Copy link

jit-ci bot commented Mar 2, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds async parity with the sync SearchIndex.query() API by enabling SQLQuery execution through AsyncSearchIndex.query(), along with dependency and documentation updates.

Changes:

  • Extend AsyncSearchIndex.query() to accept and execute SQLQuery via sql-redis async executor/registry.
  • Bump sql-redis optional dependency to >=0.2.0 (and refresh lockfile).
  • Add integration tests and user-guide notebook updates demonstrating async SQL usage.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
redisvl/index/index.py Adds async SQL execution path and updates async query typing/docs to include SQLQuery.
tests/integration/test_sql_redis_json.py Adds async integration coverage for SQL SELECT and aggregate COUNT queries.
pyproject.toml Updates the sql-redis extra (and all) to require >=0.2.0.
uv.lock Lockfile refresh to reflect dependency/version updates.
docs/user_guide/12_sql_to_redis_queries.ipynb Adds an “Async Support” section demonstrating AsyncSearchIndex.query(SQLQuery(...)).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add async SQL query execution using sql-redis AsyncSchemaRegistry and AsyncExecutor
- Update sql-redis dependency to >=0.2.0
- Add integration tests for async SQL SELECT and AGGREGATE queries
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1168 to 1172
handles post-processing of the search.

Args:
query (Union[BaseQuery, AggregateQuery, HybridQuery]): The query to run.
query (Union[BaseQuery, AggregationQuery, HybridQuery]): The query to run.

Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The SearchIndex.query() docstring type annotation for query doesn’t include SQLQuery, but the method signature accepts it (and there’s an isinstance(query, SQLQuery) branch). Please update the docstring union/type description to include SQLQuery for accuracy.

Copilot uses AI. Check for mistakes.
Comment on lines +2111 to +2115
registry = AsyncSchemaRegistry(client)
await registry.load_all() # Loads index schemas from Redis asynchronously

executor = AsyncExecutor(client, registry)

Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

AsyncSearchIndex._sql_query() builds a new AsyncSchemaRegistry, calls load_all(), and constructs a new AsyncExecutor on every SQL query. This adds extra Redis round-trips per query and can significantly reduce the concurrency/latency benefits this feature is aiming for. Consider caching the registry/executor on the index instance (lazy-init once) or loading only the current index schema if sql-redis supports it, with an explicit refresh path when needed.

Suggested change
registry = AsyncSchemaRegistry(client)
await registry.load_all() # Loads index schemas from Redis asynchronously
executor = AsyncExecutor(client, registry)
# Lazily initialize and cache the schema registry and executor to avoid
# re-loading schemas and re-creating the executor on every SQL query.
registry = getattr(self, "_sql_schema_registry", None)
executor = getattr(self, "_sql_executor", None)
cached_client = getattr(self, "_sql_client_for_sql", None)
# (Re)create the registry/executor if they don't exist yet or if the
# underlying client has changed.
if registry is None or executor is None or cached_client is not client:
registry = AsyncSchemaRegistry(client)
# Loads index schemas from Redis asynchronously; do this once per
# registry/client instead of once per query.
await registry.load_all()
executor = AsyncExecutor(client, registry)
# Cache for reuse in subsequent SQL queries.
self._sql_schema_registry = registry
self._sql_executor = executor
self._sql_client_for_sql = client

Copilot uses AI. Check for mistakes.
Comment on lines +1098 to +1104
"# Create async index\n",
"async_index = AsyncSearchIndex.from_dict(schema, redis_url=\"redis://localhost:6379\")\n",
"\n",
"# Execute SQL query asynchronously\n",
"sql_query = SQLQuery(f\"SELECT user, age FROM {async_index.name} WHERE age > 30\")\n",
"results = await async_index.query(sql_query)\n",
"\n",
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The new Async example creates an AsyncSearchIndex and immediately runs a SQL query, but it never calls await async_index.create(...) / loads data. As written, the snippet only works if the earlier sync section already created the index and loaded documents, which isn’t obvious when reading this section in isolation. Please either (a) add a short note that it reuses the previously-created index/data, or (b) include the minimal async setup steps so the section is self-contained.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@rbs333 rbs333 left a comment

Choose a reason for hiding this comment

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

Tested locally - runs great 👍 Great add!

decoded_rows.append(decoded_row)

return decoded_rows
return [convert_bytes(row) for row in result.rows]
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️

@nkanu17 nkanu17 merged commit 5ac1ab6 into main Mar 2, 2026
55 checks passed
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.

Add SQLQuery support to AsyncSearchIndex.query()

3 participants