Skip to content

Fixes #35071: filter CockroachDB schema comments by pg_namespace classoid - #35133

Merged
ulixius9 merged 1 commit into
mainfrom
detail/bug-fix/fixes-35071-filter-cockroachdb-schema-comments-by-2653da
Oct 15, 2026
Merged

ulixius9 merged 1 commit into
mainfrom
detail/bug-fix/fixes-35071-filter-cockroachdb-schema-comments-by-2653da

Conversation

@detail-app

@detail-app detail-app Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Describe your changes:

Fixes #35071

I worked on the CockroachDB connector's schema-description ingestion because COCKROACH_SCHEMA_COMMENTS published incorrect (fabricated or overwritten) schema descriptions.

Bug & root cause. The query joined pg_namespace to pg_description on objoid = n.oid alone, with no classoid filter, and put d.objsubid = 0 in the WHERE clause. PostgreSQL's pg_description contract is that (objoid, classoid, objsubid) together identify a described object. On PostgreSQL this happens to work because bootstrap data assigns non-overlapping OID ranges across system catalogs; CockroachDB's emulated catalog does not — pg_namespace.oid for public collides with pg_proc.oid of built-in functions (e.g. regr_avgy, regr_intercept). Two defects interacted:

  • Missing classoid: unrelated pg_proc doc-comments were returned as schema comments.
  • objsubid = 0 in WHERE (not ON): collapsed the LEFT JOIN into an effective INNER JOIN, so even un-commented schemas got a fabricated comment (e.g. on a fresh instance, defaultdb.public ingested the regr_avgy doc-string instead of None). When a real comment existed, the consumer's last-row-wins loop overwrote it with the bogus function comment.

Fix. Moved both predicates into the ON clause and added the classoid filter, matching the shape of POSTGRES_SCHEMA_COMMENTS in postgres/queries.py:

LEFT JOIN pg_description d
  ON n.oid = d.objoid
 AND d.classoid = 'pg_namespace'::regclass
 AND d.objsubid = 0;

This restricts matches to real namespace comments (root cause) and restores LEFT JOIN semantics so un-commented schemas return NULL. The consumer is unchanged — with the fix, pg_description returns at most one row per namespace, so the last-row-wins loop can no longer observe duplicates.

Introduced in commit 39dcb5b (the CockroachDB connector, which ported the Postgres query but misplaced objsubid and dropped classoid).

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change
  • Documentation

High-level design:

N/A — small change (one SQL constant in queries.py plus regression tests).

Tests:

Use cases covered

  • A CockroachDB schema with a real COMMENT ON SCHEMA ingests its real description, not a colliding pg_proc doc-comment.
  • A schema with no namespace comment ingests no description (None), not a fabricated function doc-comment.
  • The query stays a LEFT JOIN so every schema is returned exactly once.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files added/updated:
    • ingestion/tests/unit/source/database/cockroach/test_queries.py (new) — 5 query-shape regression tests guarding the classoid filter presence, ON-vs-WHERE placement of both predicates, no WHERE clause, and that it stays a LEFT JOIN.
    • ingestion/tests/unit/topology/database/test_cockroach.py — 4 consumer tests for set_schema_description_map/get_schema_description (previously zero coverage on this path): the real comment is kept (not a function comment), an un-commented schema yields None, the read path resolves the map correctly, and the consumer executes the fixed query constant.
  • Reverting the fix makes the 5 query-shape tests fail (and the consumer "executes the fixed query" test), confirming they are meaningful regressions.
  • Routine checks pass: unit tests (nox -s unit-tests), ruff lint, ruff format, basedpyright on the changed src/ files (0 errors), and the import-layer guard (0 broken contracts).

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • I ran the integration tests covering my changed files locally (PR CI runs only unit tests; the merge queue runs tests/integration/).
    • Reproduced the bug against a fresh CockroachDB v24.1.0 container (the version from the report): with the buggy query, defaultdb.public returned the regr_avgy doc-comment, and a database with a real comment returned two rows (real + bogus regr_intercept). With the fixed query, Scenario A returns one row with description = NULL and Scenario B returns exactly one row with the real comment.
    • Ran the actual set_schema_description_map consumer logic (the real COCKROACH_SCHEMA_COMMENTS constant + the real dict-building loop) against the live v24.1.0 instance across both scenarios — the real comment is preserved and no fabricated function comment leaks through.
    • The full tests/integration/cockroach/test_metadata.py workflow test could not be completed in this sandbox: bringing up the OpenMetadata server via ./docker/run_local_docker.sh -m no-ui -i false triggers a full Maven build (no prebuilt jar exists), which was killed by the sandbox memory limit during the swagger-maven-plugin resolution step on openmetadata-service. The test fails at fixture setup with Connection refused to localhost:8585, confirming the only blocker is the unbuilt HTTP server. The targeted end-to-end checks above exercise the same source constant and consumer loop against the real database and cover the bug's guarantees.

Playwright (UI) tests

  • Not applicable (no UI changes).

Manual testing performed

  • Started cockroachdb/cockroach:v24.1.0 via Docker, created a database with COMMENT ON SCHEMA public IS 'my real schema comment';, and ran the buggy vs fixed query by hand to observe the OID collision (objoid=105 matched both pg_proc and pg_namespace) and its resolution.

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.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • 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. For complex issues, comment the issue number in the test for future reference.

Automatic Fixes PRs can be configured here.

@detail-app
detail-app Bot requested a review from a team as a code owner October 10, 2026 02:13
@detail-app
detail-app Bot requested a review from akashverma0786 October 10, 2026 02:13
@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!

@detail-app detail-app Bot added the safe to test Add this label to run secure Github workflows on PRs label Oct 10, 2026
@sonarqubecloud

Copy link
Copy Markdown

@ulixius9
ulixius9 added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 7df74f9 Oct 15, 2026
108 of 117 checks passed
@ulixius9
ulixius9 deleted the detail/bug-fix/fixes-35071-filter-cockroachdb-schema-comments-by-2653da branch October 15, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Detail Bug] CockroachDB ingestion publishes incorrect schema descriptions due to pg_description OID collisions

2 participants