Skip to content

Conversation

@itsankit-google
Copy link
Contributor

@itsankit-google itsankit-google force-pushed the PLUGIN-994 branch 2 times, most recently from d9bef18 to 85948ac Compare December 6, 2021 18:07
@Override
public Map<String, String> getConnectionArguments() {
Map<String, String> arguments = new HashMap<>(super.getConnectionArguments());
// If connected to MySQL > 5.0.2, and setFetchSize() > 0 on a statement,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should go into getDBSpecificArguments. Also Please add a check for fetch size. If for whatever reason it's <=0 we should not add this argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@itsankit-google itsankit-google requested a review from tivv December 10, 2021 19:09

@Override
public Integer getFetchSize() {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we add proper fetchSize here?

Copy link
Contributor Author

@itsankit-google itsankit-google Dec 14, 2021

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@tivv tivv left a comment

Choose a reason for hiding this comment

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

Does this cover CloudSQL plugins?

@tivv
Copy link
Contributor

tivv commented Dec 14, 2021

I see that CloudSQL plugins descent from io.cdap.plugin.db.batch.source.AbstractDBSource.DBSourceConfig and that one has hard coded "0" as fetch size

@itsankit-google
Copy link
Contributor Author

Yes, it does cover CloudSQL plugins as discussed offline.

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.

2 participants