fix(trino): don't ingest an unset column comment as an empty description

[harshsoni2024]

Describe your changes:

Fixes #31544

• I worked on the connector half of Trino: empty column comments ingested as "", and updated column comments ignored with overrideMetadata=true #31544: Trino columns with no comment were
  ingested with an empty-string description.

• Trino's SHOW COLUMNS reports a column with no comment as the empty string,
  never NULL — and it reports an explicit COMMENT '' identically, so the two
  are indistinguishable at that layer. _get_columns passed record.Comment
  straight through, and sql_column_handler maps it onto description, so every
  uncommented column landed in OpenMetadata with "description": "" rather than no
  description at all.

• Normalizing '' to None is enough to fix it: the sink serializes with
  exclude_none=True, so the field is now omitted from the payload entirely.

• Applied the same normalization to the table comment in get_table_comment.
  TRINO_TABLE_COMMENTS filters "comment" is not null but not COMMENT '', so a
  table commented with an empty string had the same problem.

• Why this matters beyond cosmetics: a blank description counts as empty on the
  server side, so it is the one state a bot PUT is allowed to overwrite. That made
  the sync look functional — an uncommented column would accept its first comment
  — while a column with any real description silently stopped updating. The
  server-side half of that behaviour is fixed separately on branch
  trino_column_description_update_issue; both are needed to close Trino: empty column comments ingested as "", and updated column comments ignored with overrideMetadata=true #31544.

Type of change:

• Bug fix

High-level design:

N/A — small change.

Tests:

Use cases covered

• A Trino column created without a comment is ingested with no description,
  rather than an empty-string description
• A Trino column created with COMMENT '' is likewise ingested with no
  description
• A Trino column with a real comment still carries it through unchanged

Unit tests

• I added unit tests for the new/changed logic.
• File updated: ingestion/tests/unit/topology/database/test_trino_metadata.py —
  new TestTrinoColumnComments class covering _get_columns:
  • test_unset_comment_is_none — '' → None
  • test_populated_comment_is_preserved — real comment unchanged
  • test_null_comment_is_none — None → None
• pytest ingestion/tests/unit/topology/database/test_trino_metadata.py →
  16 passed.
• ruff check and ruff format --check clean on both changed files.

Backend integration tests

• Not applicable (no backend changes on this branch).

Ingestion integration tests

• Not applicable — covered by the unit tests above plus the manual
  end-to-end run below. Trino integration tests require a live Trino instance.

Playwright (UI) tests

• Not applicable (no UI changes).

Manual testing performed

Local Trino (memory catalog) against a local OpenMetadata stack.

1. Created tables covering all three comment states:

   CREATE TABLE memory.testdb.sample_table (
       id BIGINT COMMENT 'Unique identifier for the record',
       age INTEGER COMMENT '',
       created_at TIMESTAMP COMMENT 'Timestamp when the record was created'
   );
   CREATE TABLE memory.testdb.student_data (
       id BIGINT,
       name VARCHAR COMMENT 'Name of the student'
   );

2. Ran metadata ingest -c trino.yaml and fetched the columns from the API.
3. Before: age and student_data.id came back as "description": "".
4. After: neither column has a description field, while id, created_at
   and name keep their comment text unchanged.

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 #31544 above.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: not applicable, no schema changes.
• For UI changes: not applicable.
• I have added tests (unit) and listed them above.

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

Greptile Summary

The PR normalizes empty Trino column and table comments to None, preventing blank descriptions from being emitted during ingestion.

• Converts empty SHOW COLUMNS comments to absent column descriptions while preserving populated comments.
• Applies equivalent normalization to cached table comments.
• Adds focused unit coverage for empty, populated, and null column comments.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking test-coverage gap for the new table-comment normalization.

The normalization matches Trino’s string-or-null comment contract and existing connector conventions; the only accepted concern is that the parallel table-comment behavior is not directly tested.

Files Needing Attention: ingestion/src/metadata/ingestion/source/database/trino/metadata.py, ingestion/tests/unit/topology/database/test_trino_metadata.py

Important Files Changed

Filename	Overview
ingestion/src/metadata/ingestion/source/database/trino/metadata.py	Correctly normalizes empty Trino comments, but the new table-comment branch lacks direct regression coverage.
ingestion/tests/unit/topology/database/test_trino_metadata.py	Adds focused coverage for all relevant column-comment states without testing the parallel table-comment change.

_{Reviews (1): Last reviewed commit: "fix(trino): don't ingest an unset column..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Ingestion Framework

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[gitar-bot]

Code Review ✅ Approved

Normalizes empty string column and table comments to None in the Trino connector so unset comments are omitted from the payload instead of ingesting as empty descriptions. 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 👍 / 👎 | Powered by Gitar — free for open source}

[greptile-apps]

Table normalization lacks coverage

The new empty-string normalization in get_table_comment is not exercised by the accompanying tests, so a regression that restores blank table descriptions would not be detected alongside the equivalent column cases.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

✅ Playwright Results — workflow succeeded

Validated commit cff55556effcc11e3a439daf1da9934b38d18c1e in Playwright run 32021306947, attempt 1.

✅ 110 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 46m 15s

⏱️ Max setup 3m 11s · max shard execution 12m 11s · max shard-job elapsed before upload 18m 37s · reporting 4s

🌐 212.99 requests/attempt · 1.79 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

• Browser traffic was 212.99 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 1.79 per UI scenario (216 boots / 121 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	46	0	0	0	0	0
✅ Shard ingestion-01	30	0	0	0	0	0
✅ Shard ingestion-02	34	0	0	0	0	0

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[github-actions]

🚦 Removed from the merge queue — failed_checks (2026-08-17T16:03:27Z)

Blocked the queue: py-tests-status

• Postgresql PR Playwright E2E Tests — gate
• TypeScript Type Generation — generate-types
• py-tests — py-tests-status, python / Verify Expected Jobs, python / Authorize & Detect Changes