Skip to content

fix: add explicit AS aliases to all columns in get_tests & populate_test_alerts_query to support ClickHouse < 24.3#2231

Open
maanh96 wants to merge 1 commit into
elementary-data:masterfrom
maanh96:master
Open

fix: add explicit AS aliases to all columns in get_tests & populate_test_alerts_query to support ClickHouse < 24.3#2231
maanh96 wants to merge 1 commit into
elementary-data:masterfrom
maanh96:master

Conversation

@maanh96
Copy link
Copy Markdown

@maanh96 maanh96 commented May 14, 2026

Fixes #2228

Description

On ClickHouse versions < 24.3, the old query analyzer prefixes all joined/CTE columns with their source table name (e.g., failed_tests.alert_id, invocations.job_id) instead of plain column names. This causes two separate failures:

  1. edr monitor crashes with Object of type Undefined is not JSON serializable in populate_test_alerts macro
  2. edr report crashes with ValidationError: field required schema_name / test_params from get_tests macro

Both have the same root cause — ClickHouse < 24.3 prefixing all joined/CTE column names with their source table name.

Changes

  • elementary/monitor/dbt_project/macros/alerts/population/test_alerts.sql — added explicit AS aliases to all table.column references in the final SELECT of populate_test_alerts_query
  • elementary/monitor/dbt_project/macros/get_tests.sql — added explicit AS aliases to all table.column references for the same reason

Root Cause

ClickHouse's old analyzer (default in versions < 24.3) prefixes all joined/CTE columns with their source table name. The new analyzer (default from 24.3+) handles this correctly. This change is fully backwards compatible — redundant AS aliases are valid SQL on all warehouses and all ClickHouse versions.

Testing

Verified on ClickHouse 23.4:

  • edr monitor now runs successfully and sends alerts without error
  • edr report now generates the report without validation errors

Summary by CodeRabbit

  • Refactor
    • Improved column naming and explicit aliasing in data queries to ensure more consistent and unambiguous alert and test metadata in outputs, reducing potential mismatches in displayed alert/test details.

Review Change Stack

@github-actions
Copy link
Copy Markdown
Contributor

👋 @maanh96
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 this pull request.

@maanh96 maanh96 requested a deployment to elementary_test_env May 14, 2026 07:50 — with GitHub Actions Waiting
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1d9915cc-9e5f-446f-8612-568ffccbc16e

📥 Commits

Reviewing files that changed from the base of the PR and between 51f421b and 80bd692.

📒 Files selected for processing (2)
  • elementary/monitor/dbt_project/macros/alerts/population/test_alerts.sql
  • elementary/monitor/dbt_project/macros/get_tests.sql
✅ Files skipped from review due to trivial changes (1)
  • elementary/monitor/dbt_project/macros/get_tests.sql
🚧 Files skipped from review as they are similar to previous changes (1)
  • elementary/monitor/dbt_project/macros/alerts/population/test_alerts.sql

📝 Walkthrough

Walkthrough

This PR adds explicit AS aliases to selected columns in two dbt SQL macros so the queries return plain column names (not table-qualified names) across ClickHouse versions: populate_test_alerts_query and get_tests().

Changes

SQL Column Aliasing for ClickHouse Compatibility

Layer / File(s) Summary
Test alerts query column aliasing
elementary/monitor/dbt_project/macros/alerts/population/test_alerts.sql
populate_test_alerts_query final SELECT DISTINCT now explicitly aliases columns from failed_tests (e.g., failed_tests.alert_id as alert_id, data_issue_id, test_execution_id, test_unique_id, model_unique_id, timestamps, schema/table/column, alert_type/sub_type, alert_description, alert_results_query, owners, tags, status, result_rows).
Test retrieval query column aliasing
elementary/monitor/dbt_project/macros/get_tests.sql
get_tests() SELECT clause now explicitly aliases test and model metadata and last_test_results fields (e.g., database_name, schema_name, description, package_name, original_path, test_params, tags, last result fields).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hop through ClickHouse, neat and spry,

Aliases laid out, no prefixes to spy.
Plain names returned, the rows sing clear,
No Undefined errors for any ear.
Hooray for tidy columns — hop, cheer, cheer! 🥕

🚥 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 and specifically describes the main change: adding explicit AS aliases to columns in two macros to fix ClickHouse < 24.3 compatibility.
Linked Issues check ✅ Passed The PR directly addresses issue #2228 by adding explicit AS aliases to all unaliased columns in populate_test_alerts_query and get_tests, matching the stated objective to fix ClickHouse < 24.3 prefixing issues.
Out of Scope Changes check ✅ Passed All changes are directly scoped to addressing the ClickHouse < 24.3 column aliasing issue; no unrelated modifications are present in the file summaries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

…est_alerts_query to support ClickHouse < 24.3
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.

populate_test_alerts_query returns prefixed column names on old ClickHouse versions, causing "Object of type Undefined is not JSON serializable" error

1 participant