Skip to content

Fix #626: Use explicit DuckDB connections for thread-safe operations#627

Merged
javihern98 merged 2 commits intomainfrom
cr-626
Mar 25, 2026
Merged

Fix #626: Use explicit DuckDB connections for thread-safe operations#627
javihern98 merged 2 commits intomainfrom
cr-626

Conversation

@javihern98
Copy link
Copy Markdown
Contributor

Summary

Fixes #626

Aggregation._agg_func() and Analytic.analyticfunc() used duckdb.query(), which operates on DuckDB's shared default in-process connection. This connection is not thread-safe — concurrent calls from multiple threads cause PendingRequest errors or deadlocks.

Replaced duckdb.query(query).to_df() with explicit per-call duckdb.connect(database=":memory:") connections in both operators, matching the pattern already used by Eval._execute_query().

Checklist

  • Code quality checks pass (ruff format, ruff check, mypy)
  • Tests pass (pytest)
  • Documentation updated (if applicable)

Impact / Risk

  • Breaking changes? None — internal implementation change only, no API changes
  • Data/SDMX compatibility concerns? None
  • Notes for release/changelog? DuckDB operations are now thread-safe for concurrent run() calls

Notes

  • Added new tests/Concurrency/test_thread_safety.py with 5 tests covering concurrent aggregation, analytic, eval, mixed, and stress scenarios
  • Eval._execute_query() already used explicit connections — no changes needed there
  • Bug was reproduced: unfixed code hangs indefinitely when two threads run aggregation concurrently

Replace duckdb.query() (shared default connection) with explicit
per-call duckdb.connect(":memory:") in Aggregation and Analytic
operators to prevent PendingRequest errors in multi-threaded usage.
@javihern98 javihern98 marked this pull request as ready for review March 25, 2026 10:40
@javihern98 javihern98 requested review from a team and mla2001 March 25, 2026 10:40
Copy link
Copy Markdown
Contributor

@mla2001 mla2001 left a comment

Choose a reason for hiding this comment

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

Looks fine! 😊

@javihern98 javihern98 merged commit 3a69fc1 into main Mar 25, 2026
20 checks passed
@javihern98 javihern98 deleted the cr-626 branch March 25, 2026 11:51
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.

Ensure duckdb queries are thread-safe

2 participants