[BUGFIX] Narrow single-pass column_values.unique on SQLAlchemy (Redshift WLM)

[leodrivera]

[BUGFIX] Narrow single-pass column_values.unique on SQLAlchemy (Redshift WLM)

Problem

On column-store backends (observed on Amazon Redshift), expect_column_values_to_be_unique against large wide tables (~120 columns including JSON / SUPER fields) is cancelled by the WLM low_timeout rule:

psycopg2.errors.InternalError_: Query (…) cancelled by WLM abort action of Query Monitoring Rule "low_timeout".
code: 1078

Root cause

The SQLAlchemy implementation of column_values.unique previously expanded into:

SELECT col NOT IN (
  SELECT col FROM source WHERE … GROUP BY col HAVING count(col) > 1
)

Redshift planned the NOT IN semi-join as two full scans of the source table. Wall time on the user's table consistently exceeded the WLM low_timeout threshold.

Attempt history

Attempt 1 — Window-function single-pass (commit edd207e6)

Replaced NOT IN (dup_subquery) with col IN (SELECT col FROM (SELECT col, count() OVER (PARTITION BY col) AS rc FROM source) WHERE rc > 1). Redshift could now scan the source exactly once.

Result: Failures dropped sharply but did not go to zero. WLM low_timeout still occasionally tripped on the same query.

Attempt 2 — Single-pass windowed selectable mirroring compound_columns.unique (commit de58111688)

Refactored to the same shape used by compound_columns.unique: a column_values.count_per_value function metric materializes a windowed subquery (SELECT *table_columns, count() OVER PARTITION BY col AS _num_rows FROM source) and the condition becomes a simple _num_rows < 2 comparison. The MySQL/SingleStore temp-table workaround was removed (no longer needed — source is referenced once).

Result: After a week of production traffic, only a single WLM cancellation occurred, but the failure mode persisted on the very wide table (~120 columns, several of which are SUPER / JSON / long-VARCHAR).

Diagnosis after Attempt 2

The remaining failure mode is not a double-scan; it is a wide-row window problem.

The windowed subquery projected every source column (the compound_columns.unique pattern requires this so that downstream unexpected_rows / unexpected_index_list paths can read original columns from the same selectable). The Redshift window operator therefore had to carry ~120 columns — including JSON SUPER fields — through partition redistribution and sort. On non-DISTKEY columns this translates into shipping thousands of bytes per row across slices, plus a wide sort that can spill to disk and trip low_timeout.

Confirmed by inspecting the actual SQL captured in the user's error trace: the inner subquery's projection lists every column of the source table.

Attempt 3 (this PR) — Narrow windowed selectable + join-back for row-retrieval

Two changes that together address the wide-row window without re-introducing the double-scan:

1. column_values.count_per_value projects only the target column and _num_rows.
   The window operator now sorts narrow rows (1 column + the running count) regardless of source-table width. This is the fast path served to unexpected_count, unexpected_values, and unexpected_value_counts.

2. unexpected_rows and unexpected_index_list are overridden to use a narrow GROUP BY col HAVING count(*) >= 2 dup-keys subquery joined back to the source table. These paths are only exercised when the caller requests SUMMARY / COMPLETE result_format; the default BASIC path is unaffected. The dup-keys subquery itself reads only the target column, and the join back to source reads only the columns actually returned to the user.

The is_sqlalchemy_metric_selectable registry keeps column_values.unique (the condition metric name) so that the framework's standard auxiliary methods correctly read their FROM clause from the narrow windowed subquery. The previously added column_values.count_per_value entry (Attempt 2) was redundant and is removed.

Result: Running in production for the last 2 weeks against the same wide table that triggered Attempts 1 and 2. Zero WLM low_timeout cancellations. The expectation has executed flawlessly across every scheduled run in that window.

SQL shape after this PR

result_format path	Generated shape	Source scans	Window row width
unexpected_count (default BASIC)	SELECT SUM(CASE WHEN _num_rows >= 2 THEN 1 ELSE 0 END) FROM (SELECT col, count() OVER (PARTITION BY col) AS _num_rows FROM source)	1	1 column
unexpected_values, unexpected_value_counts	SELECT col [, COUNT(*)] FROM (… narrow windowed subquery …) WHERE _num_rows >= 2 [GROUP BY col]	1	1 column
unexpected_rows (SUMMARY / COMPLETE)	SELECT t.* FROM source t JOIN (SELECT col FROM source WHERE col IS NOT NULL GROUP BY col HAVING count(*) >= 2) d ON t.col = d.col	2 (narrow agg + join)	n/a
unexpected_index_list	SELECT idx_cols, t.col FROM source t JOIN (… narrow dup-keys agg …) d ON t.col = d.col	2	n/a

Before this PR, every path paid the wide-row window cost. After this PR, the window is always narrow, and the only multi-scan path is row-retrieval which is opt-in (SUMMARY / COMPLETE) and reads narrowly.

Files changed

• great_expectations/expectations/metrics/column_map_metrics/column_values_unique.py — narrow windowed selectable; subclass override of _register_metric_functions to replace the SQLAlchemy unexpected_rows and unexpected_index_list aux methods.
• great_expectations/expectations/metrics/map_metric_provider/is_sqlalchemy_metric_selectable.py — keep column_values.unique registered (Attempt 2 also added column_values.count_per_value; redundant and removed).
• tests/expectations/metrics/test_core.py — replaces the PARTITION BY-only regression assertion with two regressions:
  1. unexpected_count SQL must contain PARTITION BY, must not contain NOT IN, and must not project any non-target source column through the windowed subquery (regression guard against the wide-row failure mode from Attempt 2).
  2. unexpected_rows SQL must use a GROUP BY / HAVING dup-keys subquery joined back to the source.

Compatibility / non-goals

• Pandas and Spark implementations are unchanged.
• MySQL / SingleStore temp-table workaround from the previous implementation is dropped — the narrow window-function path references source once and works natively on MySQL 8+ / SingleStore (both support window functions). Develop's [BUGFIX] Restore SQLAlchemy 1.4 compatibility in column_values_unique (fixes #11875) #11876 patched that workaround for SQLA 1.4; the fix is moot here since the workaround no longer exists, but the same Select import pattern is applied to our remaining isinstance checks.
• SQLAlchemy 1.4 compatible — isinstance checks use Select from great_expectations.compatibility.sqlalchemy (consistent with [BUGFIX] Restore SQLAlchemy 1.4 compatibility in column_values_unique (fixes #11875) #11876).
• No Expectation API surface changes; only the generated SQL changes.
• DISTKEY on the target column further reduces shuffle but is no longer required to stay inside low_timeout on the observed table.

Operational note for users hitting this on Redshift

If you are on a wide table and expect_column_values_to_be_unique is still slow after this fix, verify that the target column is the table's DISTKEY (or at least not random-distributed). The narrow window shipped here makes shuffle volume scale with row count rather than row width, but DISTKEY on the partition column avoids the shuffle entirely.

[netlify]

👷 Deploy request for niobium-lead-7998 pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	dc2c097

[github-actions]

Is this PR still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity.

It will be closed if no further activity occurs. Thank you for your contributions 🙇

[netlify]

👷 Deploy request for niobium-lead-7998 pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	3b15662

[leodrivera]

Not stale

[joshua-stauffer]

this only projects named table_columns. When a row_condition is present, get_domain_records returns a Select, so the if branch takes it as-is and wraps it in .subquery(). That subquery's .c collection is not keyed by the original column names, so source_selectable.c['row_id'] raises KeyError, re-raised as MetricResolutionError.

[leodrivera]

Fixed in 294a33b — _named_source_subquery now always wraps the domain selectable in an explicit named projection; when get_domain_records returns a SELECT * ... WHERE Select it is converted to a subquery first, so .c is populated in both shapes. Covered by test_complete_with_row_condition_and_unexpected_index_column_names_sql and test_include_unexpected_rows_with_row_condition_sql from #11964.

[joshua-stauffer]

hey @leodrivera - thanks for taking the time to look into this. This is a tricky bug, and i appreciate your thorough notes on it. On review I noticed some gaps with how the validation result is created under certain result formats that will need to be addressed prior to merge. I wrote some tests in #11964 which might be helpful, if you want to pull them in.

There's also an issue with a metric warning that gets raised noisily on every import great_expectations, since you're overriding a previously registered metric. This is a great use case to register a custom provider, so I'll look into reworking the base class to resolve the registration error.

[joshua-stauffer]

this breaks the unexpected_index_query in data docs, because we didn't override _sqlalchemy_map_condition_query in the map metric provider to handle the new path.

[leodrivera]

Fixed in 294a33b — registered a dedicated _sqlalchemy_unique_unexpected_index_query provider for the .unexpected_index_query suffix. It builds the same join-back query as the other row-retrieval paths and renders it with sqlalchemy_select_to_sql_string, so the string surfaced in validation results and Data Docs runs as-is against the source database. Covered by test_unexpected_index_query_is_executable_sql from #11964.

[joshua-stauffer]

this breaks the canonical shape of [{row_id: [1, 2, 3, 4]}]

[leodrivera]

Fixed in 294a33b — dropped the local reimplementation and reused _get_sqlalchemy_customized_unexpected_index_list, so exclude_unexpected_values=True returns the canonical columnar [{index_column: [values, ...]}] shape. Covered by test_exclude_unexpected_values_returns_columnar_index_list_sql from #11964.

[leodrivera]

Thanks for the review and the regression tests in #11964 — I pulled them into this branch unchanged and used them to drive the fixes (commit 294a33b). Status per point:

1. row_condition KeyError (_named_source_subquery) — Fixed. The helper now always wraps the selectable in an explicit named projection; when get_domain_records returns a SELECT * ... WHERE Select it is converted to a subquery first, so .c is populated in both shapes.

2. unexpected_index_query in Data Docs — Fixed. Registered a dedicated _sqlalchemy_unique_unexpected_index_query provider using the same join-back pattern as the other row-retrieval paths, rendered via sqlalchemy_select_to_sql_string. The emitted query now runs as-is against the source database (covered by test_unexpected_index_query_is_executable_sql).

3. exclude_unexpected_values shape — Fixed. Dropped the local reimplementation and reused _get_sqlalchemy_customized_unexpected_index_list, so the canonical columnar [{index_column: [values, ...]}] shape is preserved.

4. Metric re-registration warnings on import — Left open on purpose. Since the registration of the auxiliary providers is hardcoded in MapMetricProvider._register_metric_functions, there's currently no clean extension point, and you mentioned you'd rework the base class to support custom providers. I'll rebase once that lands — test_import_does_not_emit_metric_reregistration_warnings is expected to stay red until then (now 3 warnings, since the unexpected_index_query override adds one).

All other tests from #11964 pass locally on SQLite; ruff check/format clean. develop has also been merged into the branch.

[joshua-stauffer]

thanks @leodrivera. I've merged a supporting change to make overriding metric implementations straightforward in #11998

[leodrivera]

Adopted the provider hooks from #11998 in 0185ea0 — the _register_metric_functions override is gone, replaced by the three sqlalchemy_*_provider class attributes. import great_expectations is now warning-free and test_import_does_not_emit_metric_reregistration_warnings passes. I also updated test__column_values_unique__sqlalchemy_row_retrieval_providers_are_generic → ..._are_narrow as anticipated in its docstring, so it now guards against reverting to the generic wide-row providers. That closes out all four review points — ready for another look.

[joshua-stauffer]

Nice, thanks for making the changes @leodrivera. Looks like mypy is failing; you can run locally using invoke types to debug.

Heads up that I'll be out next week; I anticipate reviewing and merging this the first week of August, and can likely include it in that week's release.

Thanks!

[leodrivera]

Fixed — invoke types pointed at three loosely-typed call sites (get_sqlalchemy_selectable / select_from / fetchall); annotated them in c593d87 with the same type: ignore + FIXME CoP markers used by the equivalent generic providers in map_condition_auxilliary_methods.py, since the proper fix lives in the signatures of those core APIs.

While at it, I ran the full static-analysis suite locally (invoke lint / invoke fmt --check / invoke types / invoke marker-coverage / check_linter_ignores.sh) — the ignores check flagged seven pre-existing bare # noqa directives in this PR's files, so 3b15662 adds the required explanatory comments. Everything is clean locally now; CI just needs a maintainer retry to get past the fork-PR permission gate.

No rush on the re-review — enjoy the time off!