Fix: render execution_time through a per-test generator instead of patching the shared dialect - #6031
Open
niklasdohmen wants to merge 1 commit into
Open
Conversation
… patch ModelTest froze time by patching the dialect's generator TRANSFORMS and SQLGlot's cached dispatch table, which are process-global, under a lock that only covers the model and CTE renders. setUp (fixture CREATE VIEW) and tearDown render outside that lock, so with concurrent_tasks > 1 another test could observe the dispatch table mid-restore and fail with "Unsupported expression type Create", or silently render CURRENT_* at the other test's frozen time. Build the frozen-time transforms into a per-test generator subclass and render SQL model tests through it; keep the lock for time_machine only. Python model tests still patch the shared dialect under the lock because their SQL goes through the engine adapter. Signed-off-by: Niklas Dohmen <niklas@enam.co>
niklasdohmen
force-pushed
the
fix/thread-safe-execution-time-rendering
branch
from
September 7, 2026 14:14
15ba5cc to
55d2c86
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ModelTestimplementsexecution_timeby patching two process-global dictionaries on the dialect's generator class —TRANSFORMSand SQLGlot's cached dispatch table — inside_concurrent_render_context. The lock there covers only the model and CTE renders;setUp(fixtureCREATE VIEW) andtearDownrender SQL outside it.With
concurrent_tasks > 1and at least one test that setsexecution_time, another thread can hit the window in whichpatch.dictrestores the dispatch table (clear, then refill) and fail withwhile creating its fixture views. Less visibly, any other thread that renders
CURRENT_DATE/CURRENT_TIMESTAMPduring the patched window gets the first test's frozen time. We saw the crash on a 106-test Databricks project running 16-wide and had to serialise the whole suite (15 minutes instead of about 1).Fix
_frozen_time_generator_class, cached per generator/time/dialect) instead of patching the shared class. SQLGlot caches one dispatch table per generator class, so the subclass gets its own and the shared one is never touched._generate_sql)._concurrent_render_contextkeeps the lock only for what is genuinely process-global:time_machine.travel.patch_shared_dialect=True). That path is unchanged in behaviour.Test
test_freeze_time_does_not_patch_shared_generator: inside a frozen test's render context, another thread rendersCREATE VIEWandCURRENT_DATEthrough the sharedduckdbdialect 200 times; asserts no error, that the shared dialect still rendersCURRENT_DATE, that the test's own generator renders the frozen cast, and that the sharedTRANSFORMSand dispatch table are byte-identical afterwards. Onmainthe shared dialect renders the frozen cast inside the context, which this test rejects; passes with this change.tests/core/test_test.pyotherwise unchanged in outcome.