Skip to content

[#4195] improvement(core): Decouple OperationDispatcher from NormalizeDispatcher#4196

Merged
jerryshao merged 1 commit intoapache:mainfrom
mchades:decouple-ops
Jul 19, 2024
Merged

[#4195] improvement(core): Decouple OperationDispatcher from NormalizeDispatcher#4196
jerryshao merged 1 commit intoapache:mainfrom
mchades:decouple-ops

Conversation

@mchades
Copy link
Contributor

@mchades mchades commented Jul 18, 2024

What changes were proposed in this pull request?

  • Decouple OperationDispatcher from NormalizeDispatcher
  • move getCatalogCapability method from OperationDispatcher to CapabilityHelpers

Why are the changes needed?

Fix: #4195

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

@mchades mchades requested a review from jerryshao July 18, 2024 06:37
@mchades mchades self-assigned this Jul 18, 2024
@jerryshao
Copy link
Contributor

I cannot get the purpose of this refactoring, can you please explain more about it?

Comment on lines +39 to +41
private final SchemaDispatcher dispatcher;

private final SchemaOperationDispatcher dispatcher;

public SchemaNormalizeDispatcher(SchemaOperationDispatcher dispatcher) {
public SchemaNormalizeDispatcher(SchemaDispatcher dispatcher, CatalogManager catalogManager) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The key to refactoring is here, replacing SchemaOperationDispatcher with SchemaDispatcher.

In the decorator design pattern, we should depend on interface SchemaDispatcher instead of the concrete implementation class SchemaOperationDispatcher.
@jerryshao

@jerryshao jerryshao merged commit d672145 into apache:main Jul 19, 2024
@mchades mchades deleted the decouple-ops branch November 22, 2024 07:32
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.

[Improvement] NormalizeDispatcher should wrap OperationDispatcher

2 participants