close
Skip to content

[Fix][Connector-V2] Fix kafka database name#9201

Merged
Hisoka-X merged 3 commits into
apache:devfrom
corgy-w:dev-kafka
Apr 22, 2025
Merged

[Fix][Connector-V2] Fix kafka database name#9201
Hisoka-X merged 3 commits into
apache:devfrom
corgy-w:dev-kafka

Conversation

@corgy-w

@corgy-w corgy-w commented Apr 18, 2025

Copy link
Copy Markdown
Contributor

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@corgy-w corgy-w marked this pull request as ready for review April 18, 2025 09:42
@nielifeng nielifeng requested a review from Copilot April 21, 2025 02:13

Copilot AI left a comment

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.

Pull Request Overview

This pull request fixes an issue with the Kafka connector’s database naming by refactoring the table path construction logic. Key changes include:

  • Extraction of the table path resolution into a new method (getTablePathFromSchema) for improved flexibility.
  • Updates to the catalog table creation to use the new table path resolution.
  • Addition of a new unit test (testDeserializationWithSchema) to validate deserialization behavior when a schema is provided.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
seatunnel-connectors-v2/connector-kafka/src/test/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfigTest.java Added a unit test to verify deserialization with a provided schema and table identifier.
seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfig.java Refactored the table path construction by extracting logic into getTablePathFromSchema to fix database naming issues.
Comments suppressed due to low confidence (2)

seatunnel-connectors-v2/connector-kafka/src/main/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfig.java:265

  • Ensure that the schema configuration key is used consistently across the connector. You might consider aligning the usage of KafkaSourceOptions.SCHEMA and TableSchemaOptions.SCHEMA to prevent potential misconfiguration.
readonlyConfig.getOptional(TableSchemaOptions.SCHEMA)

seatunnel-connectors-v2/connector-kafka/src/test/java/org/apache/seatunnel/connectors/seatunnel/kafka/source/KafkaSourceConfigTest.java:86

  • [nitpick] Consider clarifying the test table identifier format (e.g., separating database and table parts) to ensure it aligns with production expectations and avoids ambiguity.
schema.put(TABLE.key(), "test.test");

@hailin0 hailin0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

waiting for ci passed

@Hisoka-X Hisoka-X merged commit 79d9a93 into apache:dev Apr 22, 2025
@corgy-w corgy-w deleted the dev-kafka branch January 28, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants