Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix][Jdbc Connector]: Fix JdbcCatalogOptions BASE builder will be changed by call required() #6788

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Jetiaime
Copy link
Contributor

@Jetiaime Jetiaime commented May 2, 2024

Purpose of this pull request

JdbcCatalogOptions BASE builder will be changed by call required() because it just a static variable and Builder inner list will be changed if you are using code like this:

OceanBaseCatalogFactory factory = new OceanBaseCatalogFactory();
factory.optionRule();
factory.optionRule();

So change the BASE variable to a function, when we call the base() function, it will create a new Builder, so it will be new every call;

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Check list

…ged by call `required()`

JdbcCatalogOptions BASE builder will be changed by call `required()` because it just a static variable and Builder inner list will be changed if you are using code like this:

```java
OceanBaseCatalogFactory factory = new OceanBaseCatalogFactory();
factory.optionRule();
factory.optionRule();
```
So change the BASE variable to a function, when we call the `base()` function, it will create a new Builder, so it will be new every call;
@Hisoka-X
Copy link
Member

Hisoka-X commented May 6, 2024

Hi @Jetiaime , thanks for open this PR! Please follow the guide to open CI on your fork repository. https://github.com/apache/seatunnel/pull/6788/checks?check_run_id=24501297956

@Hisoka-X
Copy link
Member

Hisoka-X commented May 6, 2024

Overall LGTM

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

Successfully merging this pull request may close these issues.

None yet

2 participants