Skip to content

APP-1316: add BigQuery expiration to elementary test temp tables#1030

Merged
MikaKerman merged 6 commits into
masterfrom
app-1316-bigquery-add-expiration-to-elementary-test-temp-tables
Jun 25, 2026
Merged

APP-1316: add BigQuery expiration to elementary test temp tables#1030
MikaKerman merged 6 commits into
masterfrom
app-1316-bigquery-add-expiration-to-elementary-test-temp-tables

Conversation

@MikaKerman

@MikaKerman MikaKerman commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a 1-hour expiration_timestamp to BigQuery test temp tables at CREATE time as a safety net against orphan accumulation when on_run_end cleanup fails.

Instead of a separate ALTER TABLE macro, threads an expiration_hours parameter through the existing infrastructure:

create_elementary_test_table
  → create_or_replace(false, relation, sql, expiration_hours=1)
    → edr_create_table_as(..., expiration_hours=1)
      → bigquery__edr_get_create_table_as_sql(...)
        → CREATE TABLE ... OPTIONS(expiration_timestamp=... INTERVAL 1 hour)

On BigQuery, when expiration_hours is set and temporary is false, the CREATE statement includes the expiration option — single API call, no separate ALTER needed. Other adapters silently ignore the parameter via Jinja kwargs.

Why not just change temporary=false to temporary=true? On Snowflake/Postgres/Redshift, temporary=true creates session-scoped temp tables that aren't visible across dbt threads — test tables created during parallel test execution wouldn't be readable from on_run_end which runs on a different connection.

Resolves APP-1316

Link to Devin session: https://app.devin.ai/sessions/2a53f516ec2142da97e0e3e0f75f0489
Requested by: @MikaKerman

Summary by CodeRabbit

  • New Features
    • Added expiration_hours support for creating tables, including through create-or-replace, and threaded through adapter implementations.
    • Where supported, table creation can include an expiration clause (notably BigQuery).
  • Tests
    • Temporary test tables are now created with expiration_hours=1, ensuring they expire after 1 hour.

Add a 12-hour expiration_timestamp to test temp tables created by
create_elementary_test_table on BigQuery. This acts as a safety net
when on_run_end cleanup fails due to transient errors (e.g. rate
limiting), preventing orphan table accumulation.

Uses a new dispatched macro set_test_table_expiration that is a no-op
for all adapters except BigQuery, where it runs ALTER TABLE SET OPTIONS.

Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>
@MikaKerman MikaKerman self-assigned this Jun 24, 2026
@linear

linear Bot commented Jun 24, 2026

Copy link
Copy Markdown

APP-1316

@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown
Contributor

👋 @MikaKerman
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in the elementary repository.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b440baf8-5095-4aec-9c3d-10ca58afa059

📥 Commits

Reviewing files that changed from the base of the PR and between a9fbcfe and 39c5b5d.

📒 Files selected for processing (2)
  • macros/edr/tests/test_utils/create_elementary_test_table.sql
  • macros/utils/table_operations/create_table_as.sql
🚧 Files skipped from review as they are similar to previous changes (1)
  • macros/edr/tests/test_utils/create_elementary_test_table.sql

📝 Walkthrough

Walkthrough

The PR threads expiration_hours through the table creation macros, updates BigQuery CREATE TABLE AS SQL to emit an expiration timestamp when provided, and sets expiration_hours=1 in the elementary test table macro.

Changes

Expiration Hours Plumbing

Layer / File(s) Summary
Macro forwarding and test call
macros/utils/table_operations/create_or_replace.sql, macros/edr/tests/test_utils/create_elementary_test_table.sql
create_or_replace and its adapter overrides accept and forward expiration_hours, and create_elementary_test_table passes expiration_hours=1 to the wrapper.
Table AS dispatch and BigQuery SQL
macros/utils/table_operations/create_table_as.sql
edr_create_table_as and edr_get_create_table_as_sql accept expiration_hours, and the BigQuery adapter emits options(expiration_timestamp=...) when temporary or when expiration_hours is set.
Remaining adapter signatures
macros/utils/table_operations/create_table_as.sql
postgres__, redshift__, databricks__, clickhouse__, duckdb__, trino__, spark__, and fabricspark__ edr_get_create_table_as_sql macros accept expiration_hours, and the Fabricspark delegation forwards it to spark__edr_get_create_table_as_sql.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A bunny hopped through macros bright,
with expiration hours tucked in tight.
BigQuery sparkled, set in place,
and test tables stayed across dbt space.
🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding BigQuery expiration support for elementary test temp tables.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch app-1316-bigquery-add-expiration-to-elementary-test-temp-tables

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@macros/edr/tests/test_utils/set_test_table_expiration.sql`:
- Line 12: The `set_test_table_expiration` test utility is invoking
`elementary.run_query` for a BigQuery DDL statement, which should not lowercase
returned column names. Update the `run_query` call in
`set_test_table_expiration.sql` to pass `lowercase_column_names=False` for the
`expiration_query` path so the `ALTER TABLE` wrapper behaves correctly for
DDL/DML results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 261e3095-b620-4525-9dd3-34f0ca2309b2

📥 Commits

Reviewing files that changed from the base of the PR and between 4cd5ca9 and 65fa84e.

📒 Files selected for processing (2)
  • macros/edr/tests/test_utils/create_elementary_test_table.sql
  • macros/edr/tests/test_utils/set_test_table_expiration.sql

Comment thread macros/edr/tests/test_utils/set_test_table_expiration.sql Outdated
devin-ai-integration Bot and others added 2 commits June 24, 2026 21:54
Instead of a separate dispatch macro + ALTER TABLE to set expiration
on BigQuery test tables, thread an expiration_hours parameter through
the existing create_or_replace → edr_create_table_as →
edr_get_create_table_as_sql chain.

BigQuery's implementation adds OPTIONS(expiration_timestamp=...) at
CREATE time when expiration_hours is set, avoiding a second API call.
Other adapters silently ignore the parameter.

TTL changed from 12 hours to 1 hour to match existing patterns.

Refs: APP-1316
Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>
dbt's Jinja does not silently accept extra kwargs - all dispatched
implementations must declare the parameter even if they don't use it.

Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@macros/utils/table_operations/create_or_replace.sql`:
- Around line 1-5: Update the adapter-specific overrides of create_or_replace so
they accept expiration_hours consistently with the base macro. The current
create_or_replace dispatcher forwards expiration_hours, but redshift__,
postgres__, spark__, fabricspark__, athena__, trino__, clickhouse__, and
duckdb__ still only take temporary, relation, and sql_query, which will break
when a non-default value is passed. Add expiration_hours (or a kwargs catch-all)
to each of those macro signatures and keep the forwarded call chain aligned with
create_or_replace.

In `@macros/utils/table_operations/create_table_as.sql`:
- Around line 24-30: The dispatched create-table-as implementations are missing
the new expiration_hours kwarg, so edr_get_create_table_as_sql will break on
adapters that only accept temporary, relation, and sql_query. Update every
dispatched macro implementation referenced by
adapter.dispatch("edr_get_create_table_as_sql", "elementary")—including
default__, snowflake__, postgres__, redshift__, databricks__, clickhouse__,
duckdb__, trino__, spark__, and fabricspark__—to accept expiration_hours (or a
catch-all **kwargs) and thread it through consistently where needed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9ff75cb6-31be-4f2e-b91d-7753f507de3e

📥 Commits

Reviewing files that changed from the base of the PR and between 65fa84e and 3cee715.

📒 Files selected for processing (3)
  • macros/edr/tests/test_utils/create_elementary_test_table.sql
  • macros/utils/table_operations/create_or_replace.sql
  • macros/utils/table_operations/create_table_as.sql

Comment thread macros/utils/table_operations/create_or_replace.sql
Comment thread macros/utils/table_operations/create_table_as.sql
Same fix as previous commit - dbt Jinja requires all dispatched
implementations to declare keyword arguments that the dispatcher passes.

Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
macros/utils/table_operations/create_table_as.sql (1)

143-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Forward expiration_hours in Fabric Spark delegation.

fabricspark__edr_get_create_table_as_sql accepts expiration_hours but drops it when delegating to spark__edr_get_create_table_as_sql, which breaks the new plumbing contract on this adapter path.

Suggested fix
 {% macro fabricspark__edr_get_create_table_as_sql(
     temporary, relation, sql_query, expiration_hours=none
 ) %}
     {{
         return(
             elementary.spark__edr_get_create_table_as_sql(
-                temporary, relation, sql_query
+                temporary, relation, sql_query, expiration_hours=expiration_hours
             )
         )
     }}
 {% endmacro %}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@macros/utils/table_operations/create_table_as.sql` around lines 143 - 150,
The fabricspark__edr_get_create_table_as_sql macro is dropping the
expiration_hours argument when delegating to
elementary.spark__edr_get_create_table_as_sql, which breaks the adapter
contract. Update the delegation in fabricspark__edr_get_create_table_as_sql so
it forwards expiration_hours along with temporary, relation, and sql_query,
keeping the signature and downstream behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@macros/utils/table_operations/create_table_as.sql`:
- Around line 143-150: The fabricspark__edr_get_create_table_as_sql macro is
dropping the expiration_hours argument when delegating to
elementary.spark__edr_get_create_table_as_sql, which breaks the adapter
contract. Update the delegation in fabricspark__edr_get_create_table_as_sql so
it forwards expiration_hours along with temporary, relation, and sql_query,
keeping the signature and downstream behavior consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 35786502-92fc-4256-a8d7-67e89879385d

📥 Commits

Reviewing files that changed from the base of the PR and between 3cee715 and 0b68aa6.

📒 Files selected for processing (1)
  • macros/utils/table_operations/create_table_as.sql

Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>
{# Create the table if it doesnt exist #}
{%- do elementary.create_or_replace(false, temp_table_relation, sql_query) %}
{%- do elementary.create_or_replace(
false, temp_table_relation, sql_query, expiration_hours=1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a comment here that this table needs to survive cross-session, so it can't be temp, but we can set expiration hours for adapters that support it (currently only BQ)

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.

Added in 39c5b5d.

Co-Authored-By: mika@elementary-data.com <mika.kerman@gmail.com>
@MikaKerman MikaKerman enabled auto-merge June 25, 2026 12:14
@MikaKerman MikaKerman merged commit 244f9db into master Jun 25, 2026
30 checks passed
@MikaKerman MikaKerman deleted the app-1316-bigquery-add-expiration-to-elementary-test-temp-tables branch June 25, 2026 12:27
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.

2 participants