Skip to content

Conversation

@kokorin
Copy link
Contributor

@kokorin kokorin commented Nov 5, 2025

When dbt user has no permissions for a table, Elementary schema_changes_from_baseline test fails with unclear message (at least in Snowflake):

001003 (42000): SQL compilation error:
syntax error line 44 at position 12 unexpected ')'.

It happens because dbt swallows DB error in adapter.get_columns_in_relation() when a user has not enough permissions.

It our case it was hard to detect because of Snowflake Secondary roles, which allowed human users to read from source DB, but not for CI users.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced schema monitoring validation to detect when model attributes cannot be properly identified, providing clear diagnostic messages instead of silent failures.

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

Added error handling to detect empty columns in a data monitoring macro. When no columns are found for a model_relation, the macro now raises a compiler error with an informative message. Query construction logic remains unchanged.

Changes

Cohort / File(s) Change Summary
Error Handling Guard
macros/edr/data_monitoring/schema_changes/get_columns_snapshot_query.sql
Adds guard clause to check if extracted columns list is empty; raises compiler error with message "Failed to detect columns for {{ model_relation }}. Ensure it exists and authorized." if no columns detected.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Focused, single-file change with straightforward error handling logic
  • Guard clause is a standard defensive programming pattern
  • No complex logic density or multi-pattern changes

Poem

🐰 A sentinel stands guard at the gate,
Checking for columns, never too late,
If none appear, an error takes flight,
With messages clear, everything's right!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title contains a typo ('Raize' instead of 'Raise') but accurately describes the main change: adding error handling when columns cannot be detected.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f92c3ac and fa2737e.

📒 Files selected for processing (1)
  • macros/edr/data_monitoring/schema_changes/get_columns_snapshot_query.sql (2 hunks)
🔇 Additional comments (2)
macros/edr/data_monitoring/schema_changes/get_columns_snapshot_query.sql (2)

17-22: Excellent error handling for missing columns.

The guard clause is well-placed and prevents the confusing Snowflake syntax error by failing early with an actionable message. This directly addresses the issue where permission denial causes adapter.get_columns_in_relation() to return an empty list, which would otherwise generate invalid SQL at line 44 (empty cross join).


101-101: Whitespace stripping adjustment.

Minor formatting change for consistency with dbt conventions. No functional impact.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

👋 @kokorin
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.

@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin had a problem deploying to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Failure
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin had a problem deploying to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Failure
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env November 5, 2025 10:42 — with GitHub Actions Inactive
@NoyaOffer
Copy link
Contributor

Thanks @kokorin , this looks great!
I'll have one of our engineers check it out.

@arbiv
Copy link
Contributor

arbiv commented Nov 11, 2025

approved, pending test

@kokorin kokorin had a problem deploying to elementary_test_env November 20, 2025 13:18 — with GitHub Actions Failure
@kokorin kokorin temporarily deployed to elementary_test_env December 4, 2025 09:50 — with GitHub Actions Inactive
@kokorin kokorin temporarily deployed to elementary_test_env December 4, 2025 09:50 — with GitHub Actions Inactive
@elazarlachkar elazarlachkar merged commit 73fe78c into elementary-data:master Dec 4, 2025
86 of 90 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.

4 participants