Skip to content

[fix](dbt-doris): the commands dbt docs generate/serve fail due to wrong function signature#60779

Open
Lastv25 wants to merge 4 commits intoapache:masterfrom
Lastv25:fix/dbt_doris_adapter_breacks_for_docs_generation
Open

[fix](dbt-doris): the commands dbt docs generate/serve fail due to wrong function signature#60779
Lastv25 wants to merge 4 commits intoapache:masterfrom
Lastv25:fix/dbt_doris_adapter_breacks_for_docs_generation

Conversation

@Lastv25
Copy link
Copy Markdown

@Lastv25 Lastv25 commented Feb 17, 2026

Removed methods related to catalog retrieval and filtering schemas from the Doris adapter implementation. Use default dbt-adapter functions.

What problem does this PR solve?

Problem Summary:

The dbt docs ... commands do not work with the dbt-doris adapter due to the error TypeError: DorisAdapter.get_catalog() takes 2 positional arguments but 3 were given . The signature of the get_catalog function changed in this commit dbt-labs/dbt-adapters@5d90ff9 . There is no need to redifine the get_catalog function in the adapter (same with _catalog_filter_table and _catalog_filter_schema functions), the default ones work.

Solution:

  • Remove functions get_catalog, _catalog_filter_schema and _catalog_filter_table from adatpter
  • Use default functions from the dbt-adapter module

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • [] Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

Removed unused methods related to catalog retrieval and filtering schemas from the Doris adapter implementation. Use default dbt-adapter functions
@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman morningman self-assigned this Feb 24, 2026
@morningman
Copy link
Copy Markdown
Contributor

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR removes overridden get_catalog, _catalog_filter_schemas, and _catalog_filter_table methods from the Doris dbt adapter, deferring to the base adapter defaults. The motivation is correct — the overridden get_catalog signature is incompatible with dbt-core 1.10+.

However, I found the following issues:

Critical

  1. Bug: Missing raise keyword for CompilationError — The original code called dbt.exceptions.raise_compiler_error(...), which is a function that internally raises the exception. The replacement CompilationError(...) merely constructs the exception object but never raises it. This means the validation check in _get_one_catalog is silently broken — if len(schemas) != 1, execution will continue past the guard clause instead of aborting. The fix is raise CompilationError(...).

Minor

  1. Unused import and variableAdapterLogger is imported and logger = AdapterLogger(__name__) is defined, but logger is never used anywhere in this file. These should be removed to keep the code clean.

Checkpoints

  • Correctness: FAIL — missing raise keyword is a bug that silently swallows an error condition.
  • Unused code: WARN — logger and AdapterLogger import are unused.
  • Removal of methods: OK — removing get_catalog, _catalog_filter_schemas, _catalog_filter_table is appropriate since the base adapter provides compatible defaults.
  • Import cleanup: OK — removed imports that were only used by the deleted methods.

if len(schemas) != 1:
dbt.exceptions.raise_compiler_error(
CompilationError(
f"Expected only one schema in Doris _get_one_catalog, found " f"{schemas}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing raise keyword. This line constructs a CompilationError object but does not raise it. The error condition will be silently ignored and execution will fall through to super()._get_one_catalog(...) even when len(schemas) != 1.

The original code used dbt.exceptions.raise_compiler_error(...) which is a function that raises internally. The replacement needs an explicit raise:

            raise CompilationError(
                f"Expected only one schema in Doris _get_one_catalog, found " f"{schemas}"
            )

from dbt_common.exceptions import CompilationError

from dbt.adapters.events.logging import AdapterLogger

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: AdapterLogger is imported and logger is assigned (line 49) but never used anywhere in this file. Consider removing both the import and the logger variable to keep the code clean.

@morningman
Copy link
Copy Markdown
Contributor

hi @Lastv25 PTAL at reviews

@morningman
Copy link
Copy Markdown
Contributor

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR removes custom get_catalog, _catalog_filter_schemas, and _catalog_filter_table methods from the Doris dbt adapter to rely on base adapter defaults, fixing a TypeError caused by a signature change in dbt-adapters.

Critical Checkpoints

Goal & Correctness: The goal is to fix dbt docs generate/serve commands. While removing the custom get_catalog is the right approach, the remaining _get_one_catalog override has an incompatible signature with the base class, which will cause a runtime failure. This is a blocking issue.

Modification Scope: The change is focused and minimal — only one file modified, removing overridden methods. Good.

Concurrency: Not applicable.

Lifecycle / Static Init: Not applicable.

Configuration Items: Not applicable.

Incompatible Changes: Yes — see critical issue below regarding _get_one_catalog signature mismatch.

Parallel Code Paths: Not applicable.

Test Coverage: PR mentions unit test, but no test changes are included in the diff. Given this is a bug fix removing overrides, manual testing of dbt docs generate should be sufficient.

Observability: Not applicable.

Performance: Not applicable.

Issues Found

  1. [CRITICAL] _get_one_catalog signature mismatch — The remaining _get_one_catalog override uses manifest: Manifest as the third parameter, but the base class BaseAdapter._get_one_catalog expects used_schemas: FrozenSet[Tuple[str, str]]. The base get_catalog (which will now be called instead of the removed custom one) passes used_schemas to _get_one_catalog. This means a FrozenSet will be passed where Manifest is expected, causing a runtime type error. The signature and super() call must be updated to match the base class, and the Manifest import should be removed (it becomes unused).

  2. [Minor] Extra blank line — There are 3 consecutive blank lines between the imports and the Engine class (lines 45-47). PEP 8 recommends exactly 2 blank lines between top-level definitions.

information_schema: InformationSchema,
schemas: Set[str],
manifest: Manifest,
) -> agate.Table:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Signature mismatch with base class.

The base BaseAdapter._get_one_catalog signature (in dbt-adapters) is:

def _get_one_catalog(
    self,
    information_schema: InformationSchema,
    schemas: Set[str],
    used_schemas: FrozenSet[Tuple[str, str]],
) -> "agate.Table":

But this override uses manifest: Manifest as the third parameter. Since the custom get_catalog was removed, the base get_catalog will now be called, which passes used_schemas (a FrozenSet[Tuple[str, str]]) as the third argument. This will fail at runtime because this override expects a Manifest object.

Fix: Update the signature to match the base class:

def _get_one_catalog(
        self,
        information_schema: InformationSchema,
        schemas: Set[str],
        used_schemas: FrozenSet[Tuple[str, str]],
) -> agate.Table:
    if len(schemas) != 1:
        raise CompilationError(
            f"Expected only one schema in Doris _get_one_catalog, found {schemas}"
        )
    return super()._get_one_catalog(information_schema, schemas, used_schemas)

Also:

  • Add FrozenSet to the typing imports
  • Remove the now-unused from dbt.contracts.graph.manifest import Manifest import

from dbt.adapters.sql.impl import LIST_RELATIONS_MACRO_NAME, LIST_SCHEMAS_MACRO_NAME
from dbt_common.clients.agate_helper import table_from_rows
from dbt.contracts.graph.manifest import Manifest
from dbt.adapters.contracts.relation import RelationType
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Since _get_one_catalog signature should be updated to use used_schemas: FrozenSet[Tuple[str, str]] instead of manifest: Manifest, this import will become unused and should be removed.

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.

3 participants