refactor: Introduce ConnectorNames.h in connectors #156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yingsu00
force-pushed
the
ConnectorNames
branch
from
ade6742 to
8bd2832
Compare
|
|
yingsu00
force-pushed
the
ConnectorNames
branch
2 times, most recently
from
2bfb849 to
775ce9d
Compare
In order to decouple Hive and other specific connectors from exec and core modules, we need to put the connector names in a central location at connectors/ConnectorNames.h. The idea is similar to dwio::common:: FileFormat where all file formats are specified. Modules outside of the connectors module can just reference this central and connector- agnostic header instead of connector specific headers like HiveConnector.h.
ZacBlanco
force-pushed
the
ConnectorNames
branch
from
775ce9d to
43c4efe
Compare
Collaborator
|
@yingsu00 I rebased your PR to get it ready to merge but looks like there are some failures in CI now. Could you take a look? Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Today, the Hive connector’s internal structures are widely referenced across unrelated modules (e.g., exec/tests, dwio, and others). This creates an undesirable coupling and violates the design principle of modular connector boundaries. Before introducing the new Iceberg connector, we should refactor the connector layer to establish a clean, common connector interface that external components depend on, rather than exposing or leaking implementation details of a specific connector.
Type of Change
Description
In order to decouple Hive and other specific connectors from exec and core modules, we need to put the connector names in a central location at connectors/ConnectorNames.h. The idea is similar to dwio::common:: FileFormat where all file formats are specified. Modules outside of the connectors module can just reference this central and connector- agnostic header instead of connector specific headers like HiveConnector.h.
Performance Impact
No Impact: This change does not affect the critical path (e.g., build system, doc, error handling).
Positive Impact: I have run benchmarks.
Click to view Benchmark Results
Negative Impact: Explained below (e.g., trade-off for correctness).
Release Note
N/A
Checklist (For Author)
Breaking Changes