Skip to content

fix(datasource): SparkConnector.table_simple_info raises TypeError on every call - #3213

Open
Anai-Guo wants to merge 1 commit into
eosphoros-ai:mainfrom
Anai-Guo:fix-spark-table-simple-info
Open

fix(datasource): SparkConnector.table_simple_info raises TypeError on every call#3213
Anai-Guo wants to merge 1 commit into
eosphoros-ai:mainfrom
Anai-Guo:fix-spark-table-simple-info

Conversation

@Anai-Guo

Copy link
Copy Markdown

Problem

SparkConnector.table_simple_info() raises TypeError on every call.

SparkConnector.get_fields is declared with a required table_name, matching the
BaseConnector.get_fields(self, table_name: str) contract
(packages/dbgpt-core/src/dbgpt/datasource/base.py:203):

def get_fields(self, table_name: str):
    """Get column meta about dataframe.

    TODO: Support table_name.
    """
    return ",".join([f"({name}: {dtype})" for name, dtype in self.df.dtypes])

but table_simple_info calls it with no arguments
(packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py:171):

def table_simple_info(self):
    """Get table simple info."""
    return f"{self.table_name}{self.get_fields()}"

self.get_fields() resolves to SparkConnector.get_fields, which needs
table_name, so the f-string never gets evaluated:

TypeError: SparkConnector.get_fields() missing 1 required positional argument: 'table_name'

Reachability

table_simple_info() is called generically on whatever connector the datasource
resolves to, so a Spark datasource hits this:

  • packages/dbgpt-serve/src/dbgpt_serve/agent/resource/datasource.py:170
    table_infos = conn.table_simple_info(), the fallback path when
    get_db_summary returns nothing.
  • packages/dbgpt-serve/src/dbgpt_serve/evaluate/service/fetchdata/benchmark_data_manager.py:1009
    return list(self._connector.table_simple_info())

Fix

Pass the table name the method already has in hand. __init__ sets
self.table_name = "temp", and the same attribute is already interpolated on the
preceding part of that very f-string.

-        return f"{self.table_name}{self.get_fields()}"
+        return f"{self.table_name}{self.get_fields(self.table_name)}"

SparkConnector.get_fields currently ignores table_name (it has an explicit
TODO: Support table_name and reads self.df.dtypes), so this changes no output
— it only lets the call succeed. When that TODO is implemented, the call site is
already passing the right thing.

Verification

No Spark cluster is needed to show this. The two method bodies were extracted from
the real file with ast and executed against a stub DataFrame:

extracted: ['get_fields', 'table_simple_info']
signature get_fields: (self, table_name: str)
UNPATCHED -> TypeError: SparkConnectorStub.get_fields() missing 1 required positional argument: 'table_name'
PATCHED   -> 'temp(id: bigint),(name: string)'

The unpatched run reproduces the exact TypeError; the patched run returns the
intended summary string.

Scope

Deliberately limited to the one broken call. Not changed here:

  • get_fields still ignores table_name — that is the pre-existing TODO, and
    implementing it is a separate change.
  • SparkConnector.table_simple_info returns a str while the RDBMS/Neo4j/TuGraph
    implementations return a sequence. That inconsistency is real but predates this
    bug and is out of scope for a crash fix.

🤖 Generated with Claude Code

…table_simple_info

SparkConnector.get_fields is declared as get_fields(self, table_name: str),
matching the BaseConnector contract, but table_simple_info called it with no
arguments, so every call raised TypeError.
@github-actions github-actions Bot added the fix Bug fixes label Aug 30, 2026
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 61798ebb-28c5-4612-9d8d-412e9d5aa810

📥 Commits

Reviewing files that changed from the base of the PR and between 0b9c505 and ec520a9.

📒 Files selected for processing (1)
  • packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
此 package 包含数据库 connector、RAG 实现、storage backend、model adapter

⚙️ CodeRabbit configuration file

Files:

  • packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py
Use Python 3.10 or newer for project development.

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Files:

  • packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py
🔇 Additional comments (1)
packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py (1)

171-171: LGTM!


📝 Walkthrough

Purpose

SparkConnector.table_simple_info() now passes self.table_name to get_fields. This prevents the missing-argument TypeError on each call. Output remains unchanged because get_fields currently ignores table_name.

Scope

  • Affects packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py.
  • No public declarations, data formats, or configuration changed.
  • The existing table-name handling TODO and return-type inconsistency remain unchanged.

Risks

  • Correctness improves by restoring the method call contract.
  • No security or performance risks are introduced.
  • Compatibility impact is limited to preventing the existing runtime failure.

Verification

No targeted test was added or reported. Add a regression test for table_simple_info() that verifies the required table name reaches get_fields.

Run the package tests and a targeted lint check:

python -m pytest packages/dbgpt-ext/tests
ruff check packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py

Walkthrough

SparkConnector.table_simple_info now passes self.table_name to get_fields, matching the method signature.

Changes

Spark connector

Layer / File(s) Summary
Pass table name to field lookup
packages/dbgpt-ext/src/dbgpt_ext/datasource/conn_spark.py
table_simple_info calls get_fields with self.table_name.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to ec520

The change fixes Spark table metadata calls that previously failed immediately by passing the existing table name to the required method, restoring the intended summary response with no actionable merge-blocking risk remaining.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commit style with the valid type fix, the datasource scope, and a concise description of the SparkConnector TypeError fix.
Description check ✅ Passed The description clearly explains the problem, root cause, fix, scope, and reproducible verification. It omits the template's Snapshots and Checklist sections and does not explicitly state dependencies…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, root cause, fix, scope, and reproducible verification. It omits the template's Snapshots and Checklist sections and does not explicitly state dependencies, but the required change context and testing details are complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant