Skip to content

feat: RYOW for DuckDB within an explicit txn and MDEV-40379 fix#5392

Open
drrtuy wants to merge 3 commits into
MariaDB:11.4from
drrtuy:bb-11.4-MDEV-40379
Open

feat: RYOW for DuckDB within an explicit txn and MDEV-40379 fix#5392
drrtuy wants to merge 3 commits into
MariaDB:11.4from
drrtuy:bb-11.4-MDEV-40379

Conversation

@drrtuy

@drrtuy drrtuy commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Add optional Read-Your-Own-Writes for cross-engine DuckDB scans

What

In cross-engine joins, DuckDB previously read non-DuckDB tables through a fiber running a synthetic SELECT in a separate background transaction, so uncommitted writes from the parent transaction were invisible. This PR adds an opt-in session variable that lets DuckDB read those uncommitted writes (RYOW) via a direct handler scan in the parent transaction.

Key changes

  • ha_duckdb.cc: new session var duckdb_cross_engine_ryow (default OFF) plus accessor get_thd_cross_engine_ryow().
  • cross_engine_scan.cc/.h: per-query RYOW toggle; new _mdb_scan_direct table function with filter_pushdown=false; replacement scan routes to _mdb_scan_direct when RYOW is on.
  • ha_duckdb_pushdown.cc: registers the RYOW flag during init_scan().
  • architecture.md: documents the RYOW path and its tradeoffs.

How to test

Run cross_engine_ryow.test: with the var OFF uncommitted parent-transaction writes to InnoDB are hidden from the join; with it ON they are visible; after ROLLBACK committed state is intact.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new session variable duckdb_cross_engine_ryow to toggle Read-Your-Own-Writes (RYOW) visibility for cross-engine joins in DuckDB, and disables temporary table support to resolve an assertion failure (MDEV-40379). The reviewer suggests a significant simplification to the RYOW implementation: instead of capturing the flag in init_scan() and storing it in a thread-local variable, the code should query current_thd directly during query compilation. This eliminates the need for the thread-local variable, its registration, cleanup logic, and an unnecessary header include, which should also be reflected in the updated documentation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread storage/duckdb/runtime/cross_engine_scan.cc
Comment thread storage/duckdb/runtime/cross_engine_scan.cc
Comment thread storage/duckdb/runtime/cross_engine_scan.cc
Comment thread storage/duckdb/runtime/cross_engine_scan.h
#include "duckdb_query.h"
#include "duckdb_context.h"
#include "cross_engine_scan.h"
#include "duckdb_config.h"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This include is no longer needed if we don't call get_thd_cross_engine_ryow here.

Comment on lines +359 to +360
myduck::register_cross_engine_ryow(myduck::get_thd_cross_engine_ryow(thd));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since the RYOW flag is now read directly from current_thd during query compilation, we don't need to register it here in init_scan().

Comment thread storage/duckdb/docs/architecture.md
@drrtuy drrtuy force-pushed the bb-11.4-MDEV-40379 branch from f326e30 to 4476c8e Compare July 16, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants