Skip to content

fix: map MONEY and BIT columns to concrete SQLAlchemy types in profiler (#29459)#30210

Open
zerafachris wants to merge 1 commit into
open-metadata:mainfrom
zerafachris:fix/mssql-money-bit-profiler-type-mapping
Open

fix: map MONEY and BIT columns to concrete SQLAlchemy types in profiler (#29459)#30210
zerafachris wants to merge 1 commit into
open-metadata:mainfrom
zerafachris:fix/mssql-money-bit-profiler-type-mapping

Conversation

@zerafachris

@zerafachris zerafachris commented Jul 19, 2026

Copy link
Copy Markdown

Fixes #29459

Root cause

CommonMapTypes._TYPE_MAP in ingestion/src/metadata/profiler/orm/converter/common.py
had no entries for DataType.MONEY or DataType.BIT. return_custom_type()
falls back to CustomTypes.UNDETERMINED.value (UndeterminedType) for any
dataType not present in the map. That fallback type:

  • is in the profiler's NOT_COMPUTE set (profiler/orm/registry.py), so profiling
    is silently skipped for these columns, and
  • renders sample data via UndeterminedType.process_result_value as the literal
    string OPENMETADATA_UNDETERMIND[value] (exactly matching the screenshots in
    the issue).

SMALLMONEY already worked because it's classified as OM DataType.NUMBER
(see column_type_parser.py), which does have a _TYPE_MAP entry. MONEY and
BIT are their own DataType enum values with no corresponding entry, so they
hit the undetermined fallback regardless of which connector reports them —
this isn't MSSQL-specific, just most commonly hit there.

What changed

  • Added DataType.MONEY: sqlalchemy.NUMERIC and DataType.BIT: sqlalchemy.BOOLEAN
    to CommonMapTypes._TYPE_MAP.
  • Mirrored both in the reverse map_sqa_to_om_types() map (merged into the
    existing sqlalchemy.NUMERIC / sqlalchemy.BOOLEAN entries rather than
    adding duplicate dict keys).

Test evidence

Added test_money_and_bit_types_are_mapped in
ingestion/tests/unit/observability/profiler/test_converter.py, building an
ORM table with MONEY and BIT columns (MSSQL service type) via
ometa_to_sqa_orm and asserting the resulting SQLAlchemy column types are
NUMERIC/BOOLEAN, not UndeterminedType. Verified fail → pass locally:

  • Before the fix: assert not isinstance(amount_type, undetermined_type)
    fails — AssertionError: assert not True.
  • After the fix: passes for both columns.

Ran the full tests/unit/observability/profiler/ suite (excluding
directories that need optional connector deps not part of the test-unit
extra — pandas, sqlalchemy_bigquery, pyathena — not installable in this
environment due to an unrelated cx_Oracle/pkg_resources build failure
under the test extra): 356 passed, 1 skipped, only pre-existing
environment-only failures unrelated to this change (missing pandas /
sqlalchemy_bigquery modules).

ruff check and ruff format --check are clean on both touched files.


Prepared with AI assistance (Claude Code, Anthropic), reviewed for correctness before submission.

Greptile Summary

This PR fixes profiling being silently skipped for MONEY and BIT columns by adding the two missing DataType entries to CommonMapTypes._TYPE_MAP and mirroring them in the reverse map_sqa_to_om_types() map. Without the fix, these columns fell through to UndeterminedType, which is in the NOT_COMPUTE set and renders sample data as the literal string OPENMETADATA_UNDETERMIND[value].

  • DataType.MONEY is now mapped to sqlalchemy.NUMERIC (forward) and included in the NUMERIC reverse-map set, enabling numeric metric computation on MONEY columns.
  • DataType.BIT is now mapped to sqlalchemy.BOOLEAN (forward) and included in the BOOLEAN reverse-map set.
  • A new unit test test_money_and_bit_types_are_mapped verifies both columns resolve to the correct SQLAlchemy types instead of UndeterminedType when building ORM tables via ometa_to_sqa_orm.

Confidence Score: 5/5

Safe to merge; the two-line change to _TYPE_MAP and matching reverse-map update are isolated and well-tested with no risk to existing mappings.

The change is minimal: two entries added to a dict, two existing sets expanded. The fix is applied in CommonMapTypes, so it is inherited by all dialect-specific subclasses (including MssqlMapTypes, AzureSqlMapTypes, etc.) without needing per-dialect changes. The new test exercises the full ORM construction path via ometa_to_sqa_orm and asserts both the positive outcome and the regression guard against UndeterminedType. No existing mappings are touched or removed.

No files require special attention.

Important Files Changed

Filename Overview
ingestion/src/metadata/profiler/orm/converter/common.py Adds DataType.MONEY → sqlalchemy.NUMERIC and DataType.BIT → sqlalchemy.BOOLEAN to both the forward _TYPE_MAP and the reverse map_sqa_to_om_types(); change is minimal and correct, with no impact on existing mappings.
ingestion/tests/unit/observability/profiler/test_converter.py Adds test_money_and_bit_types_are_mapped covering both new mappings end-to-end via ometa_to_sqa_orm with an MSSQL service type; assertions correctly guard against UndeterminedType regression.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Column with dataType MONEY or BIT"] --> B["map_types() in CommonMapTypes"]
    B --> C{"dataType in _TYPE_MAP?"}
    C -- "Before fix: NO" --> D["return_custom_type() → UndeterminedType"]
    D --> E["NOT_COMPUTE set → profiling skipped"]
    D --> F["sample data = 'OPENMETADATA_UNDETERMIND[value]'"]
    C -- "After fix: YES" --> G{"dataType match"}
    G -- "DataType.MONEY" --> H["sqlalchemy.NUMERIC → numeric metrics computed"]
    G -- "DataType.BIT" --> I["sqlalchemy.BOOLEAN → boolean profiling"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Column with dataType MONEY or BIT"] --> B["map_types() in CommonMapTypes"]
    B --> C{"dataType in _TYPE_MAP?"}
    C -- "Before fix: NO" --> D["return_custom_type() → UndeterminedType"]
    D --> E["NOT_COMPUTE set → profiling skipped"]
    D --> F["sample data = 'OPENMETADATA_UNDETERMIND[value]'"]
    C -- "After fix: YES" --> G{"dataType match"}
    G -- "DataType.MONEY" --> H["sqlalchemy.NUMERIC → numeric metrics computed"]
    G -- "DataType.BIT" --> I["sqlalchemy.BOOLEAN → boolean profiling"]
Loading

Reviews (1): Last reviewed commit: "fix(profiler): map MONEY and BIT columns..." | Re-trigger Greptile

…open-metadata#29459)

CommonMapTypes._TYPE_MAP had no entries for DataType.MONEY or DataType.BIT,
so any column classified as one of these types (e.g. MSSQL's MONEY/BIT
columns) fell back to CustomTypes.UNDETERMINED. That fallback type is in
the profiler's NOT_COMPUTE set (profiling is skipped) and renders sample
data as the literal string "OPENMETADATA_UNDETERMIND[value]" instead of
the actual value.

Map DataType.MONEY to sqlalchemy.NUMERIC (consistent with the existing
SMALLMONEY -> NUMBER -> NUMERIC path) and DataType.BIT to
sqlalchemy.BOOLEAN, and mirror both in the reverse SQA-to-OM type map.
@zerafachris
zerafachris requested a review from a team as a code owner July 19, 2026 09:06
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

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.

@github-actions

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot

gitar-bot Bot commented Jul 19, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Maps MONEY and BIT types to NUMERIC and BOOLEAN in the profiler converter, resolving silent profiling skips and incorrect data rendering. 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

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.

MSSQL money / smallmoney types not handled in sampler serialisation and profiler metric mapping

1 participant