Fixes 21482: only compare a column against '' when the type can hold it

[TeddyCr]

Describe your changes:

Fixes #21482

The columnValuesMissingCount ("Column Values Missing Count") test always compared the column against the empty string, whatever its type:

SUM(CAST(CASE WHEN (id2 IS NULL) THEN 1 WHEN (id2 = '') THEN 1 ELSE 0 END AS NUMERIC)) AS "nullCount"

On PostgreSQL an INTEGER column makes this fail outright — psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type integer: "" — so the test aborts. On MySQL it does not fail: '' is coerced to 0, so every zero row is counted as missing and the test silently reports a wrong number.

The empty-string branch comes from NullMissingCount (added in #16017 for #14812, which only considered string columns). It is now emitted only for types that can actually hold ''. The predicate fails open: quantifiable, date/time, boolean, UUID, interval, enum and IP types are excluded; anything else — including dialect-specific string types we don't recognize — keeps counting empty strings exactly as before.

CustomTime (the MariaDB TIME mapping) is listed in the metric's exclusion tuple rather than added to registry.is_date_time, because that helper also drives min/max/mean/stddev and columnValuesToBeBetween; widening it would change those paths too, which is out of scope for this fix.

missingValueMatch is unaffected: user-supplied missing values go through the separate countInSet metric.

The pandas/DataFrame path is deliberately left alone. A CSV/datalake column typed as INT genuinely can carry "" for a missing value, so guarding there would drop real missing values; the SQL path has no such case because the engine enforces the type.

Type of change:

• Bug fix

High-level design:

N/A — small change.

Tests:

Use cases covered

• Column Values Missing Count on a PostgreSQL INTEGER column runs and reports only the NULLs, instead of aborting with invalid input syntax for type integer: ""
• The same test on a MySQL INT/BOOLEAN column no longer counts 0 / false rows as missing
• The same test on a MariaDB TIME column no longer counts '00:00:00' rows as missing
• The same test on DATE, BOOLEAN, UUID, INTERVAL, ENUM and IPV4/IPV6 columns no longer errors on PostgreSQL (and on DATE for MySQL, which raised Incorrect DATE value: '' in strict mode)
• String, text and unrecognized/custom string-like columns keep counting '' as missing — no behavior change

Unit tests

• I added unit tests for the new/changed logic.
• Files added: ingestion/tests/unit/observability/profiler/sqlalchemy/test_null_missing_count_metric.py
• Compiles the real metric expression against both the PostgreSQL and MySQL dialects and asserts on the emitted SQL per column type: no = '' for Integer, SmallInteger, BIGINT, Numeric, DECIMAL, Float, Date, DateTime, Time, CustomTimestamp, CustomTime, Boolean, UUIDString, Interval, Enum, CustomIP; = '' preserved for String, VARCHAR, TEXT, CHAR, NVARCHAR and the unrecognized-type fail-open case.
• A data-driven case walks CommonMapTypes._TYPE_MAP for the 18 OM DataTypes that cannot hold '', so a converter change that reintroduces the bug fails the suite.

Backend integration tests

• Not applicable (no backend API changes).

Ingestion integration tests

• I added ingestion integration tests.
• Files added: ingestion/tests/integration/profiler/test_null_missing_count_metric.py
• Runs the metric against real PostgreSQL 15 and MySQL 8.4 testcontainers over a table with 3 NULLs, 2 zeros, 2 false and 2 empty strings, asserting integer/date/boolean columns count 3 and the string column counts 5.

Playwright (UI) tests

• Not applicable (no UI changes).

Manual testing performed

Verified the generated SQL before/after, and the engine behavior on throwaway containers with a table holding 3 NULLs and 2 zeros:

	before	after
PostgreSQL integer	ERROR: invalid input syntax for type integer: ""	3
MySQL int	5 (zeros counted as missing)	3
MySQL boolean	4 (false rows counted)	2
MariaDB time	2 ('00:00:00' counted)	1
PostgreSQL date / boolean / uuid / interval / inet / enum	ERROR: invalid input syntax for type …: ""	works
MySQL date	ERROR 1525: Incorrect DATE value: ''	works

Full ingestion/tests/unit/observability/profiler + ingestion/tests/unit/observability/data_quality suites plus the new integration file pass (819 passed, 1 skipped). The new tests were confirmed RED against the unpatched metric.

Note for reviewers — behavior change on MySQL / MariaDB

On MySQL, integer and boolean columns previously had their 0 / false rows counted as missing; on MariaDB, TIME columns had their '00:00:00' rows counted. Anyone who calibrated missingCountValue against those inflated numbers will see their test go from green to red after this fix. The new number is the correct one, but it is worth a release note.

Blast radius is limited to this DQ test: NullMissingCount is a distinct metric from the profiler's NullCount (static/null_count.py), which never had the = '' branch, so stored profiles and nullProportion history are untouched.

One deliberate trade-off: MySQL permits '' as a real ENUM member, so an enum column with an empty label will no longer have those rows counted. PostgreSQL native enums raise on the comparison, so avoiding the hard error was preferred over that edge case. Anyone affected can add "" explicitly via missingValueMatch, which is routed through countInSet and unaffected by this change.

The predicate fails open by design, so six types in supportedDataTypes that are not in CommonMapTypes._TYPE_MAP (TIMESTAMPZ, VARIANT, GEOMETRY, POINT, POLYGON, LOWCARDINALITY) still emit the comparison. That is intentional — LOWCARDINALITY(String) genuinely can hold '', and TIMESTAMPZ is unreachable from SQL ingestion because column_type_parser normalizes TIMESTAMPTZ to TIMESTAMP. If one of the exotic ones ever surfaces a point = '' report, it belongs in NON_EMPTY_STRING_TYPES.

UI screen recording / screenshots:

Not applicable.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #<issue-number> above.
• I have commented on my code, particularly in hard-to-understand areas.
• I have added tests (unit / integration / Playwright as applicable) and listed them above.

• I have added a test that covers the exact scenario we are fixing.

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• Issue Some test case errors #21482 is missing Shipping → Source.
• Issue Some test case errors #21482 is missing Shipping → Release.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[gitar-bot]

Code Review ✅ Approved

Restricts empty-string comparisons in column missing-count metrics to types that support them, preventing PostgreSQL syntax errors and false positives on numeric and boolean types. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the NullMissingCount metric to avoid comparing non-string columns against the empty string (''), preventing runtime errors on strict engines and incorrect coercion/counts on MySQL/MariaDB. It also adds unit and integration coverage to ensure the metric only checks '' for types that can meaningfully hold it.

Changes:

• Add a type guard (can_hold_empty_string) so NullMissingCount only compares = '' for string-capable types.
• Add SQL compilation unit tests across Postgres/MySQL dialects for both string and non-string ORM/OM types.
• Add integration tests using real Postgres/MySQL engines to validate correct missing counts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
ingestion/src/metadata/profiler/metrics/static/null_missing_count.py	Adds can_hold_empty_string and conditionally includes the empty-string comparison in SQL generation.
ingestion/tests/unit/observability/profiler/sqlalchemy/test_null_missing_count_metric.py	Adds dialect compilation tests ensuring = '' is excluded for non-string types and included for string-like/unknown types.
ingestion/tests/integration/profiler/test_null_missing_count_metric.py	Adds engine-level validation against Postgres/MySQL using testcontainers to ensure counts match expected semantics.

[Khairajani]

JSON and array columns can hit this too — col = '' errors on both Postgres and MySQL, so sqlalchemy.JSON, sqlalchemy.ARRAY and CustomArray could go in here as well.

Binary types are fine as-is though — an empty blob really does match '', so excluding those would drop real counts.