You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by mbode <gi...@git.apache.org> on 2017/05/31 20:06:58 UTC
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
GitHub user mbode opened a pull request:
https://github.com/apache/flink/pull/4036
[FLINK-6781] Make statement fetch size configurable in JDBCInputFormat
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mbode/flink JDBCInputFormat_fetchsize
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4036.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4036
----
commit 69d9b9faa580be626fdef970c9b2f83ed67d798a
Author: Maximilian Bode <ma...@tngtech.com>
Date: 2017-05-31T16:46:55Z
[FLINK-6781] Make statement fetch size configurable in JDBCInputFormat.
commit 34bc56b31990907a73f146ccc21e58980e214fac
Author: Maximilian Bode <ma...@tngtech.com>
Date: 2017-05-31T16:48:51Z
[FLINK-6781] Update DataSet docs concerning JDBCInputFormat.
The way to provide type information was outdated.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by mbode <gi...@git.apache.org>.
Github user mbode commented on a diff in the pull request:
https://github.com/apache/flink/pull/4036#discussion_r119528882
--- Diff: flink-connectors/flink-jdbc/src/test/java/org/apache/flink/api/java/io/jdbc/JDBCInputFormatTest.java ---
@@ -101,6 +105,33 @@ public void testIncompleteConfiguration() throws IOException {
}
@Test
+ public void defaultFetchSizeIsUsedIfNotConfiguredOtherwise() throws SQLException {
+ jdbcInputFormat = JDBCInputFormat.buildJDBCInputFormat()
+ .setDrivername(DRIVER_CLASS)
+ .setDBUrl(DB_URL)
+ .setQuery(SELECT_ALL_BOOKS)
+ .setRowTypeInfo(ROW_TYPE_INFO)
+ .finish();
+ jdbcInputFormat.openInputFormat();
+ assertThat(jdbcInputFormat.getStatement().getFetchSize(), equalTo(1));
--- End diff --
The idea was to make sure that existing behavior is not broken. I have made the dependency on the fact that derby is used in this test explicit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4036: [FLINK-6781] Make statement fetch size configurable in JD...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4036
merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4036: [FLINK-6781] Make statement fetch size configurable in JD...
Posted by mbode <gi...@git.apache.org>.
Github user mbode commented on the issue:
https://github.com/apache/flink/pull/4036
[Green Travis](https://travis-ci.org/mbode/flink/builds/238207585)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4036#discussion_r119465263
--- Diff: flink-connectors/flink-jdbc/src/test/java/org/apache/flink/api/java/io/jdbc/JDBCInputFormatTest.java ---
@@ -101,6 +105,33 @@ public void testIncompleteConfiguration() throws IOException {
}
@Test
+ public void defaultFetchSizeIsUsedIfNotConfiguredOtherwise() throws SQLException {
+ jdbcInputFormat = JDBCInputFormat.buildJDBCInputFormat()
+ .setDrivername(DRIVER_CLASS)
+ .setDBUrl(DB_URL)
+ .setQuery(SELECT_ALL_BOOKS)
+ .setRowTypeInfo(ROW_TYPE_INFO)
+ .finish();
+ jdbcInputFormat.openInputFormat();
+ assertThat(jdbcInputFormat.getStatement().getFetchSize(), equalTo(1));
+
+ }
+
+ @Test
+ public void fetchSizeCanBeConfigured() throws SQLException {
+ final int desiredFetchSize = 10_000;
+ jdbcInputFormat = JDBCInputFormat.buildJDBCInputFormat()
+ .setDrivername(DRIVER_CLASS)
+ .setDBUrl(DB_URL)
+ .setQuery(SELECT_ALL_BOOKS)
+ .setRowTypeInfo(ROW_TYPE_INFO)
+ .setFetchSize(desiredFetchSize)
+ .finish();
+ jdbcInputFormat.openInputFormat();
+ assertThat(jdbcInputFormat.getStatement().getFetchSize(), equalTo(desiredFetchSize));
--- End diff --
Would be good to replace these with junit assertEquals, to be consistent with the remaining tests in flink.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4036
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4036: [FLINK-6781] Make statement fetch size configurable in JD...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4036
Looks good, +1 to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4036#discussion_r119464888
--- Diff: flink-connectors/flink-jdbc/src/test/java/org/apache/flink/api/java/io/jdbc/JDBCInputFormatTest.java ---
@@ -101,6 +105,33 @@ public void testIncompleteConfiguration() throws IOException {
}
@Test
+ public void defaultFetchSizeIsUsedIfNotConfiguredOtherwise() throws SQLException {
+ jdbcInputFormat = JDBCInputFormat.buildJDBCInputFormat()
+ .setDrivername(DRIVER_CLASS)
+ .setDBUrl(DB_URL)
+ .setQuery(SELECT_ALL_BOOKS)
+ .setRowTypeInfo(ROW_TYPE_INFO)
+ .finish();
+ jdbcInputFormat.openInputFormat();
+ assertThat(jdbcInputFormat.getStatement().getFetchSize(), equalTo(1));
+
+ }
+
+ @Test
+ public void fetchSizeCanBeConfigured() throws SQLException {
--- End diff --
by convention the name of test methods should be prefixed with "test".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4036#discussion_r119464737
--- Diff: flink-connectors/flink-jdbc/src/test/java/org/apache/flink/api/java/io/jdbc/JDBCInputFormatTest.java ---
@@ -101,6 +105,33 @@ public void testIncompleteConfiguration() throws IOException {
}
@Test
+ public void defaultFetchSizeIsUsedIfNotConfiguredOtherwise() throws SQLException {
+ jdbcInputFormat = JDBCInputFormat.buildJDBCInputFormat()
+ .setDrivername(DRIVER_CLASS)
+ .setDBUrl(DB_URL)
+ .setQuery(SELECT_ALL_BOOKS)
+ .setRowTypeInfo(ROW_TYPE_INFO)
+ .finish();
+ jdbcInputFormat.openInputFormat();
+ assertThat(jdbcInputFormat.getStatement().getFetchSize(), equalTo(1));
--- End diff --
That is equals 1 is an internal detail of the derby jdbc driver is it not? Not sure how useful this test is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4036: [FLINK-6781] Make statement fetch size configurabl...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4036#discussion_r119464746
--- Diff: flink-connectors/flink-jdbc/src/main/java/org/apache/flink/api/java/io/jdbc/JDBCInputFormat.java ---
@@ -378,6 +388,11 @@ public JDBCInputFormatBuilder setRowTypeInfo(RowTypeInfo rowTypeInfo) {
return this;
}
+ public JDBCInputFormatBuilder setFetchSize(int fetchSize) {
+ format.fetchSize = fetchSize;
--- End diff --
let's add a `Preconditions` check that fetchSize is greater than 0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---