[jira] [Commented] (DRILL-7104) Change of data type when parquet with multiple fragment
[
https://issues.apache.org/jira/browse/DRILL-7104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801567#comment-16801567
]
benj commented on DRILL-7104:
-
A little trick to get around this problem until someone can fix it.
Use a specific string value (example : 'MYSPECIALVALUE' or '' or anything else
appropriate in your case) INSTEAD of NULL in a inner request, then use a NULLIF
function to change the value of the column in a outer SELECT.
{code:java}
/* Trick to avoid the bug that change the NULL(VARCHAR) into NULL(INT) */
CREATE TABLE `trickavoidbug` AS
(SELECT NULLIF(demo,'TRICK'), jam
FROM
((SELECT 'TRICK' AS demo
,md5(CAST(rand() AS VARCHAR)) AS jam
FROM `onebigfile` LIMIT 100)
UNION
(SELECT 'TRICK' AS demo
,md5(CAST(rand() AS VARCHAR)) AS jam
FROM `onebigfile` LIMIT 100))
);
{code}
Hopefully this will help someone who is facing this problem while waiting for
his resolution.
> Change of data type when parquet with multiple fragment
> ---
>
> Key: DRILL-7104
> URL: https://issues.apache.org/jira/browse/DRILL-7104
> Project: Apache Drill
> Issue Type: Bug
> Components: Storage - Parquet
>Affects Versions: 1.15.0
>Reporter: benj
>Priority: Major
> Attachments: DRILL-7104_ErrorNumberFormatException_20190322.log
>
>
> When creating a Parquet with a column filled only with "CAST(NULL AS
> VARCHAR)", if the parquet has several fragment, the type is read like INT
> instead of VARCHAR.
> First, create +Parquet with only one fragment+ - all is fine (the type of
> "demo" is correct).
> {code:java}
> CREATE TABLE `nobug` AS
> (SELECT CAST(NULL AS VARCHAR) AS demo
> , md5(cast(rand() AS VARCHAR) AS jam
> FROM `onebigfile` LIMIT 100));
> +---++
> | Fragment | Number of records written |
> +---++
> | 0_0 | 1000 |
> SELECT drilltypeof(demo) AS goodtype FROM `bug` LIMIT 1;
> ++
> | goodtype |
> ++
> | VARCHAR|
> {code}
> Second, create +Parquet with at least 2 fragments+ - the type of "demo"
> change to INT
> {code:java}
> CREATE TABLE `bug` AS
> ((SELECT CAST(NULL AS VARCHAR) AS demo
> ,md5(CAST(rand() AS VARCHAR)) AS jam
> FROM `onebigfile` LIMIT 100)
> UNION
> (SELECT CAST(NULL AS VARCHAR) AS demo
> ,md5(CAST(rand() AS VARCHAR)) AS jam
> FROM `onebigfile` LIMIT 100));
> +---++
> | Fragment | Number of records written |
> +---++
> | 1_1 | 1000276|
> | 1_0 | 999724 |
> SELECT drilltypeof(demo) AS badtype FROM `bug` LIMIT 1;
> ++
> | badtype|
> ++
> | INT|{code}
> The change of type is really terrible...
>
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7077) Add Function to Facilitate Time Series Analysis
[
https://issues.apache.org/jira/browse/DRILL-7077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801571#comment-16801571
]
ASF GitHub Bot commented on DRILL-7077:
---
cgivre commented on issue #1680: DRILL-7077: Add Function to Facilitate Time
Series Analysis
URL: https://github.com/apache/drill/pull/1680#issuecomment-476565199
@arina-ielchiieva Commits squashed. Thanks for the review.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Add Function to Facilitate Time Series Analysis
> ---
>
> Key: DRILL-7077
> URL: https://issues.apache.org/jira/browse/DRILL-7077
> Project: Apache Drill
> Issue Type: New Feature
>Reporter: Charles Givre
>Assignee: Charles Givre
>Priority: Major
> Labels: doc-impacting, ready-to-commit
> Fix For: 1.16.0
>
>
> When analyzing time based data, you will often have to aggregate by time
> grains. While some time grains will be easy to calculate, others, such as
> quarter, can be quite difficult. These functions enable a user to quickly and
> easily aggregate data by various units of time. Usage is as follows:
> {code:java}
> SELECT
> FROM
> GROUP BY nearestDate(, {code}
> So let's say that a user wanted to count the number of hits on a web server
> per 15 minute, the query might look like this:
> {code:java}
> SELECT nearestDate(`eventDate`, '15MINUTE' ) AS eventDate,
> COUNT(*) AS hitCount
> FROM dfs.`log.httpd`
> GROUP BY nearestDate(`eventDate`, '15MINUTE'){code}
> Currently supports the following time units:
> * YEAR
> * QUARTER
> * MONTH
> * WEEK_SUNDAY
> * WEEK_MONDAY
> * DAY
> * HOUR
> * HALF_HOUR / 30MIN
> * QUARTER_HOUR / 15MIN
> * MINUTE
> * 30SECOND
> * 15SECOND
> * SECOND
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801585#comment-16801585
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269022764
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
##
@@ -541,9 +546,9 @@ private void runPreparedStatement(final
PreparedStatementHandle preparedStatemen
ServerPreparedStatementState.PARSER.parseFrom(preparedStatementHandle.getServerInfo());
} catch (final InvalidProtocolBufferException ex) {
throw UserException.parseError(ex)
- .message("Failed to parse the prepared statement handle. " +
- "Make sure the handle is same as one returned from create
prepared statement call.")
- .build(logger);
+ .message("Failed to parse the prepared statement handle. " +
Review comment:
The initial indentation was better, please revert this change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801593#comment-16801593
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269036148
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -72,12 +73,18 @@
public class PreparedStatementTest extends JdbcTestBase {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(PreparedStatementTest.class);
+ private static final Random RANDOMIZER = new Random(20150304);
private static final String SYS_VERSION_SQL = "select * from sys.version";
private static final String SYS_RANDOM_SQL =
"SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) " +
"union SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) " +
"union SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) ";
+ private static final String SYS_OPTIONS_SQL = "SELECT * FROM sys.options";
+ private static final String SYS_OPTIONS_SQL_LIMIT_10 = "SELECT * FROM
sys.options LIMIT 12";
+ private static final String ALTER_SYS_OPTIONS_MAX_ROWS_LIMIT_X = "ALTER
SYSTEM SET `" + ExecConstants.QUERY_MAX_ROWS + "`=";
+ //Locks used across StatementTest and PreparedStatementTest
+ private static Semaphore maxRowsSysOptionLock =
StatementTest.maxRowsSysOptionLock;
Review comment:
`connection` is created in this class, but the same `Semaphore` is used from
`StatementTest` class for this test class and another one. Should they use
their own instances of `Semaphore`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801584#comment-16801584
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269016874
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
##
@@ -283,6 +300,13 @@ public RemoteFunctionRegistry getRemoteFunctionRegistry()
{
return drillbitContext.getRemoteFunctionRegistry();
}
+ /**
+ * Allows to disable autolimit in case it is not applicable
+ */
+ public void disableAutoLimit() {
Review comment:
This method also may be inlined.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801594#comment-16801594
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269039279
##
File path: exec/jdbc/src/test/java/org/apache/drill/jdbc/StatementTest.java
##
@@ -239,4 +246,173 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(Statement stmt = connection.createStatement()) {
+ int maxRowsValue = stmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ // Setting negative value
+ int valueToSet = -10;
+ try {
+stmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ // Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ stmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, stmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ stmt.setMaxRows(0);
+ stmt.executeQuery(SYS_OPTIONS_SQL);
+ ResultSet rs = stmt.getResultSet();
Review comment:
`ResultSet` should be also closed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801586#comment-16801586
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269021867
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
##
@@ -143,13 +143,18 @@ public Foreman(final WorkerBee bee, final
DrillbitContext drillbitContext,
this.closeFuture = initiatingClient.getChannelClosureFuture();
closeFuture.addListener(closeListener);
+// Apply AutoLimit on resultSet (Usually received via REST APIs)
+final int autoLimit = queryRequest.getAutolimitRowcount();
+if (autoLimit > 0) {
+
connection.getSession().getOptions().setLocalOption(ExecConstants.QUERY_MAX_ROWS,
autoLimit);
+}
this.queryContext = new QueryContext(connection.getSession(),
drillbitContext, queryId);
this.queryManager = new QueryManager(queryId, queryRequest,
drillbitContext.getStoreProvider(),
drillbitContext.getClusterCoordinator(), this);
this.queryRM = drillbitContext.getResourceManager().newQueryRM(this);
this.fragmentsRunner = new FragmentsRunner(bee, initiatingClient,
drillbitContext, this);
-this.queryStateProcessor = new QueryStateProcessor(queryIdString,
queryManager, drillbitContext, new ForemanResult());
this.profileOption = setProfileOption(queryContext.getOptions());
+this.queryStateProcessor = new QueryStateProcessor(queryIdString,
queryManager, drillbitContext, new ForemanResult());
Review comment:
Could you please explain the reason for this change?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801595#comment-16801595
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269039838
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2489CallsAfterCloseThrowExceptionsTest.java
##
@@ -537,7 +547,15 @@ public void
testclosedPreparedStmtOfOpenConnMethodsThrowRight() {
new ClosedPreparedStatementChecker(PreparedStatement.class,
closedPreparedStmtOfOpenConn);
-checker.testAllMethods();
+//List of methods now supported
+@SuppressWarnings("serial")
Review comment:
Please revert these changes in this class once calls to Avatica methods are
used.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801589#comment-16801589
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269026715
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
Review comment:
Please use `try with resources` for `Statement` instead of manually closing
it. Also, please rename `setMaxStmt` into `statement`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801596#comment-16801596
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269038543
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -462,4 +618,25 @@ public void
testParamSettingWhenUnsupportedTypeSaysUnsupported() throws SQLExcep
}
}
+
+ // Sets the SystemMaxRows option
+ private void setSystemMaxRows(int sysValueToSet) throws SQLException {
Review comment:
I'm not sure that the approach used for blocking the connection should be
implemented in this method. I think unit tests should acquire and release the
lock.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801587#comment-16801587
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269025296
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
##
@@ -379,6 +379,12 @@ private QueryProfile getQueryProfile(UserException ex) {
profileBuilder.setQuery(queryText);
}
+final int autoLimitRowCount =
foreman.getQueryContext().getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue();
Review comment:
Please remove `final` modifier here and in other places where it is not
required.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801591#comment-16801591
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269027601
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
Review comment:
Please remove this line and put into the beginning of the method call
`super.setLargeMaxRows(maxRowCount)`, so it will check that connection is open
and validate `maxRowCount` value.
The same for changes in `DrillStatementImpl`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801592#comment-16801592
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269032278
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ int maxRowsValue = pStmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting negative value
+ int valueToSet = -10;
+ try {
+pStmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ pStmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, pStmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ pStmt.setMaxRows(0);
+ pStmt.execute();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ assertTrue(rowCount > 0);
+}
+ }
+
+ /**
+ * Test setting maxSize at a value lower than existing query limit
+ */
+ @Test
+ public void testSetMaxRowsLowerThanQueryLimit() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL_LIMIT_10)) {
+ int valueToSet = RANDOMIZER.nextInt(9)+1; //range:[1-9]
+ pStmt.setMaxRows(valueToSet);
+ pStmt.executeQuery();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ logger.info("MaxRows was set as {} and query limited to 10 returned a
resultSet of {} rows", valueToSet, rowCount);
Review comment:
Please remove loggers from unit tests, it is redundant.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801588#comment-16801588
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269027913
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
+ }
+
+ @Override
+ public long getLargeMaxRows() {
Review comment:
Please remove this method since Avatica provides its implementation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801590#comment-16801590
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269029315
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
Review comment:
Please add spaces here and in other places where needed (and remove where
they are excessive, like in the signature of this test).
```suggestion
try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801597#comment-16801597
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269033910
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -462,4 +618,25 @@ public void
testParamSettingWhenUnsupportedTypeSaysUnsupported() throws SQLExcep
}
}
+
+ // Sets the SystemMaxRows option
+ private void setSystemMaxRows(int sysValueToSet) throws SQLException {
+// Acquire lock if Non-Zero Value (i.e. a test is in progress)
+if (sysValueToSet != 0) {
+ maxRowsSysOptionLock.acquireUninterruptibly();
+}
+// Setting the value
+try (Statement stmt = connection.createStatement()) {
+ stmt.executeQuery(ALTER_SYS_OPTIONS_MAX_ROWS_LIMIT_X + sysValueToSet);
+ ResultSet rs = stmt.getResultSet();
Review comment:
Is it required to obtain a result set and iterate thru it?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[ https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801583#comment-16801583 ] ASF GitHub Bot commented on DRILL-7048: --- vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC Statement.setMaxRows() with System Option URL: https://github.com/apache/drill/pull/1714#discussion_r269013774 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java ## @@ -107,6 +107,23 @@ public QueryContext(final UserSession session, final DrillbitContext drillbitCon this.table = drillbitContext.getOperatorTable(); } +// Checking for limit on ResultSet rowcount and if user attempting to override the system value +final int sessionMaxRowCount = queryOptions.getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue(); Review comment: There is no need to make these variables final. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Implement JDBC Statement.setMaxRows() with System Option > > > Key: DRILL-7048 > URL: https://issues.apache.org/jira/browse/DRILL-7048 > Project: Apache Drill > Issue Type: New Feature > Components: Client - JDBC, Query Planning & Optimization >Affects Versions: 1.15.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua >Priority: Major > Labels: doc-impacting > Fix For: 1.17.0 > > > With DRILL-6960, the webUI will get an auto-limit on the number of results > fetched. > Since more of the plumbing is already there, it makes sense to provide the > same for the JDBC client. > In addition, it would be nice if the Server can have a pre-defined value as > well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a > max limit on the resultset size as well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[ https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801983#comment-16801983 ] ASF GitHub Bot commented on DRILL-7048: --- kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC Statement.setMaxRows() with System Option URL: https://github.com/apache/drill/pull/1714#discussion_r269232507 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java ## @@ -107,6 +107,23 @@ public QueryContext(final UserSession session, final DrillbitContext drillbitCon this.table = drillbitContext.getOperatorTable(); } +// Checking for limit on ResultSet rowcount and if user attempting to override the system value +final int sessionMaxRowCount = queryOptions.getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue(); Review comment: In previous reviews, I've been asked to keep immutable variables as `final`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Implement JDBC Statement.setMaxRows() with System Option > > > Key: DRILL-7048 > URL: https://issues.apache.org/jira/browse/DRILL-7048 > Project: Apache Drill > Issue Type: New Feature > Components: Client - JDBC, Query Planning & Optimization >Affects Versions: 1.15.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua >Priority: Major > Labels: doc-impacting > Fix For: 1.17.0 > > > With DRILL-6960, the webUI will get an auto-limit on the number of results > fetched. > Since more of the plumbing is already there, it makes sense to provide the > same for the JDBC client. > In addition, it would be nice if the Server can have a pre-defined value as > well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a > max limit on the resultset size as well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801986#comment-16801986
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269234329
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
##
@@ -143,13 +143,18 @@ public Foreman(final WorkerBee bee, final
DrillbitContext drillbitContext,
this.closeFuture = initiatingClient.getChannelClosureFuture();
closeFuture.addListener(closeListener);
+// Apply AutoLimit on resultSet (Usually received via REST APIs)
+final int autoLimit = queryRequest.getAutolimitRowcount();
+if (autoLimit > 0) {
+
connection.getSession().getOptions().setLocalOption(ExecConstants.QUERY_MAX_ROWS,
autoLimit);
+}
this.queryContext = new QueryContext(connection.getSession(),
drillbitContext, queryId);
this.queryManager = new QueryManager(queryId, queryRequest,
drillbitContext.getStoreProvider(),
drillbitContext.getClusterCoordinator(), this);
this.queryRM = drillbitContext.getResourceManager().newQueryRM(this);
this.fragmentsRunner = new FragmentsRunner(bee, initiatingClient,
drillbitContext, this);
-this.queryStateProcessor = new QueryStateProcessor(queryIdString,
queryManager, drillbitContext, new ForemanResult());
this.profileOption = setProfileOption(queryContext.getOptions());
+this.queryStateProcessor = new QueryStateProcessor(queryIdString,
queryManager, drillbitContext, new ForemanResult());
Review comment:
Since I was originally changing the ForemanResult, I had reordered to ensure
that the `queryContext` was set with the right values before the
`queryStateProcessor` is initialized. I'll reorder to the original way,
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801987#comment-16801987
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269235028
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
##
@@ -541,9 +546,9 @@ private void runPreparedStatement(final
PreparedStatementHandle preparedStatemen
ServerPreparedStatementState.PARSER.parseFrom(preparedStatementHandle.getServerInfo());
} catch (final InvalidProtocolBufferException ex) {
throw UserException.parseError(ex)
- .message("Failed to parse the prepared statement handle. " +
- "Make sure the handle is same as one returned from create
prepared statement call.")
- .build(logger);
+ .message("Failed to parse the prepared statement handle. " +
Review comment:
Ok.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801989#comment-16801989
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269235600
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -462,4 +618,25 @@ public void
testParamSettingWhenUnsupportedTypeSaysUnsupported() throws SQLExcep
}
}
+
+ // Sets the SystemMaxRows option
+ private void setSystemMaxRows(int sysValueToSet) throws SQLException {
+// Acquire lock if Non-Zero Value (i.e. a test is in progress)
+if (sysValueToSet != 0) {
+ maxRowsSysOptionLock.acquireUninterruptibly();
+}
+// Setting the value
+try (Statement stmt = connection.createStatement()) {
+ stmt.executeQuery(ALTER_SYS_OPTIONS_MAX_ROWS_LIMIT_X + sysValueToSet);
+ ResultSet rs = stmt.getResultSet();
Review comment:
No. If the query fails, you won't get the resultset object.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801990#comment-16801990
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269235963
##
File path: exec/jdbc/src/test/java/org/apache/drill/jdbc/StatementTest.java
##
@@ -239,4 +246,173 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(Statement stmt = connection.createStatement()) {
+ int maxRowsValue = stmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ // Setting negative value
+ int valueToSet = -10;
+ try {
+stmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ // Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ stmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, stmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (Statement stmt = connection.createStatement()) {
+ stmt.setMaxRows(0);
+ stmt.executeQuery(SYS_OPTIONS_SQL);
+ ResultSet rs = stmt.getResultSet();
Review comment:
Ok, but `Statement.close()` also closes the `ResultSet`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801991#comment-16801991
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269236778
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -462,4 +618,25 @@ public void
testParamSettingWhenUnsupportedTypeSaysUnsupported() throws SQLExcep
}
}
+
+ // Sets the SystemMaxRows option
+ private void setSystemMaxRows(int sysValueToSet) throws SQLException {
Review comment:
The unit tests use this method to set and release the lock. It reduces the
lock code being repeated across unit tests.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16801993#comment-16801993
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269237610
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -72,12 +73,18 @@
public class PreparedStatementTest extends JdbcTestBase {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(PreparedStatementTest.class);
+ private static final Random RANDOMIZER = new Random(20150304);
private static final String SYS_VERSION_SQL = "select * from sys.version";
private static final String SYS_RANDOM_SQL =
"SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) " +
"union SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) " +
"union SELECT cast(random() as varchar) as myStr FROM (VALUES(1)) ";
+ private static final String SYS_OPTIONS_SQL = "SELECT * FROM sys.options";
+ private static final String SYS_OPTIONS_SQL_LIMIT_10 = "SELECT * FROM
sys.options LIMIT 12";
+ private static final String ALTER_SYS_OPTIONS_MAX_ROWS_LIMIT_X = "ALTER
SYSTEM SET `" + ExecConstants.QUERY_MAX_ROWS + "`=";
+ //Locks used across StatementTest and PreparedStatementTest
+ private static Semaphore maxRowsSysOptionLock =
StatementTest.maxRowsSysOptionLock;
Review comment:
I tried that, but the unit tests attempt to test with setting up specific
SYSTEM values. Without this, there is a race condition where both
`StatementTest` and `PreparedStatementTest` attempt to set SYSTEM values... and
get different results (resulting in test failures).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802007#comment-16802007
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269243737
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
##
@@ -283,6 +300,13 @@ public RemoteFunctionRegistry getRemoteFunctionRegistry()
{
return drillbitContext.getRemoteFunctionRegistry();
}
+ /**
+ * Allows to disable autolimit in case it is not applicable
+ */
+ public void disableAutoLimit() {
Review comment:
👍
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802013#comment-16802013
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269246088
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
+ }
+
+ @Override
+ public long getLargeMaxRows() {
Review comment:
👍
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802022#comment-16802022
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269249958
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
Review comment:
Ok, but I think it should be done post-close() because in case of a failure,
we dont want to accidentally set through `super` and then worry about rolling
back.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802027#comment-16802027
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269251734
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ int maxRowsValue = pStmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting negative value
+ int valueToSet = -10;
+ try {
+pStmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ pStmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, pStmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ pStmt.setMaxRows(0);
+ pStmt.execute();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ assertTrue(rowCount > 0);
+}
+ }
+
+ /**
+ * Test setting maxSize at a value lower than existing query limit
+ */
+ @Test
+ public void testSetMaxRowsLowerThanQueryLimit() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL_LIMIT_10)) {
+ int valueToSet = RANDOMIZER.nextInt(9)+1; //range:[1-9]
+ pStmt.setMaxRows(valueToSet);
+ pStmt.executeQuery();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ logger.info("MaxRows was set as {} and query limited to 10 returned a
resultSet of {} rows", valueToSet, rowCount);
Review comment:
Same as previous comment regarding `final`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802031#comment-16802031
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269252724
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2489CallsAfterCloseThrowExceptionsTest.java
##
@@ -537,7 +547,15 @@ public void
testclosedPreparedStmtOfOpenConnMethodsThrowRight() {
new ClosedPreparedStatementChecker(PreparedStatement.class,
closedPreparedStmtOfOpenConn);
-checker.testAllMethods();
+//List of methods now supported
+@SuppressWarnings("serial")
Review comment:
Not sure what you meant by the change. Should I remove the SuppressWarnings?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[ https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802032#comment-16802032 ] ASF GitHub Bot commented on DRILL-7048: --- vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC Statement.setMaxRows() with System Option URL: https://github.com/apache/drill/pull/1714#discussion_r269253834 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java ## @@ -107,6 +107,23 @@ public QueryContext(final UserSession session, final DrillbitContext drillbitCon this.table = drillbitContext.getOperatorTable(); } +// Checking for limit on ResultSet rowcount and if user attempting to override the system value +final int sessionMaxRowCount = queryOptions.getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue(); Review comment: I'm not sure that it was exactly the same case, but in this case `final` is redundant. The scope of variable usage is small, and there is no risk of accidental changing its value. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Implement JDBC Statement.setMaxRows() with System Option > > > Key: DRILL-7048 > URL: https://issues.apache.org/jira/browse/DRILL-7048 > Project: Apache Drill > Issue Type: New Feature > Components: Client - JDBC, Query Planning & Optimization >Affects Versions: 1.15.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua >Priority: Major > Labels: doc-impacting > Fix For: 1.17.0 > > > With DRILL-6960, the webUI will get an auto-limit on the number of results > fetched. > Since more of the plumbing is already there, it makes sense to provide the > same for the JDBC client. > In addition, it would be nice if the Server can have a pre-defined value as > well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a > max limit on the resultset size as well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802038#comment-16802038
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269255206
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
Review comment:
It is important to check that connection is opened at first, and it is done
in the parent method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802069#comment-16802069
]
ASF GitHub Bot commented on DRILL-7048:
---
vvysotskyi commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269263831
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ int maxRowsValue = pStmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting negative value
+ int valueToSet = -10;
+ try {
+pStmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ pStmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, pStmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ pStmt.setMaxRows(0);
+ pStmt.execute();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ assertTrue(rowCount > 0);
+}
+ }
+
+ /**
+ * Test setting maxSize at a value lower than existing query limit
+ */
+ @Test
+ public void testSetMaxRowsLowerThanQueryLimit() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL_LIMIT_10)) {
+ int valueToSet = RANDOMIZER.nextInt(9)+1; //range:[1-9]
+ pStmt.setMaxRows(valueToSet);
+ pStmt.executeQuery();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ logger.info("MaxRows was set as {} and query limited to 10 returned a
resultSet of {} rows", valueToSet, rowCount);
Review comment:
What is the purpose of this message? It will be newer read by dev except for
the case when a test failed and logging is enabled. But the real purpose may be
when this message is moved into an assertion check, so in the case of test
failure, it is printed and in other cases, logging does not happen.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802084#comment-16802084
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269273829
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
+this.maxRowCount = maxRowCount;
Review comment:
👍
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[ https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802120#comment-16802120 ] ASF GitHub Bot commented on DRILL-7048: --- kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC Statement.setMaxRows() with System Option URL: https://github.com/apache/drill/pull/1714#discussion_r269293742 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java ## @@ -107,6 +107,23 @@ public QueryContext(final UserSession session, final DrillbitContext drillbitCon this.table = drillbitContext.getOperatorTable(); } +// Checking for limit on ResultSet rowcount and if user attempting to override the system value +final int sessionMaxRowCount = queryOptions.getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue(); Review comment: 👍 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Implement JDBC Statement.setMaxRows() with System Option > > > Key: DRILL-7048 > URL: https://issues.apache.org/jira/browse/DRILL-7048 > Project: Apache Drill > Issue Type: New Feature > Components: Client - JDBC, Query Planning & Optimization >Affects Versions: 1.15.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua >Priority: Major > Labels: doc-impacting > Fix For: 1.17.0 > > > With DRILL-6960, the webUI will get an auto-limit on the number of results > fetched. > Since more of the plumbing is already there, it makes sense to provide the > same for the JDBC client. > In addition, it would be nice if the Server can have a pre-defined value as > well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a > max limit on the resultset size as well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802121#comment-16802121
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269294222
##
File path:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillPreparedStatementImpl.java
##
@@ -259,4 +261,17 @@ public void setObject(int parameterIndex, Object x,
SQLType targetSqlType) throw
checkOpen();
super.setObject(parameterIndex, x, targetSqlType);
}
+
+ @Override
+ public void setLargeMaxRows(long maxRowCount) throws SQLException {
+Statement setMaxStmt = this.connection.createStatement();
+setMaxStmt.execute("ALTER SESSION SET `" + ExecConstants.QUERY_MAX_ROWS +
"`=" + maxRowCount);
+setMaxStmt.close();
Review comment:
👍
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802127#comment-16802127
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269251734
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ int maxRowsValue = pStmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting negative value
+ int valueToSet = -10;
+ try {
+pStmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ pStmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, pStmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ pStmt.setMaxRows(0);
+ pStmt.execute();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ assertTrue(rowCount > 0);
+}
+ }
+
+ /**
+ * Test setting maxSize at a value lower than existing query limit
+ */
+ @Test
+ public void testSetMaxRowsLowerThanQueryLimit() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL_LIMIT_10)) {
+ int valueToSet = RANDOMIZER.nextInt(9)+1; //range:[1-9]
+ pStmt.setMaxRows(valueToSet);
+ pStmt.executeQuery();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ logger.info("MaxRows was set as {} and query limited to 10 returned a
resultSet of {} rows", valueToSet, rowCount);
Review comment:
~~Same as previous comment regarding `final`~~
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-7048) Implement JDBC Statement.setMaxRows() with System Option
[
https://issues.apache.org/jira/browse/DRILL-7048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802131#comment-16802131
]
ASF GitHub Bot commented on DRILL-7048:
---
kkhatua commented on pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269298181
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##
@@ -410,6 +417,155 @@ public void testNonTriggeredQueryTimeout() throws
SQLException {
}
}
+
+ // Query maxRows methods:
+
+ /**
+ * Test for reading of default max rows
+ */
+ @Test
+ public void testDefaultGetMaxRows() throws SQLException {
+try(PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ int maxRowsValue = pStmt.getMaxRows();
+ assertEquals(0, maxRowsValue);
+}
+ }
+
+ /**
+ * Test Invalid parameter by giving negative maxRows value
+ */
+ @Test
+ public void testInvalidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting negative value
+ int valueToSet = -10;
+ try {
+pStmt.setMaxRows(valueToSet);
+ } catch ( final SQLException e) {
+assertThat( e.getMessage(), containsString("VALIDATION ERROR: Option
exec.query.max_rows must be between 0 and ") );
+ }
+}
+ }
+
+ /**
+ * Test setting a valid maxRows value
+ */
+ @Test
+ public void testValidSetMaxRows() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ //Setting positive value
+ int valueToSet = RANDOMIZER.nextInt(59)+1;
+ logger.info("Setting maximum resultset size as {} rows", valueToSet);
+ pStmt.setMaxRows(valueToSet);
+ assertEquals( valueToSet, pStmt.getMaxRows() );
+}
+ }
+
+ /**
+ * Test setting maxSize as zero (i.e. no Limit)
+ */
+ @Test
+ public void testSetMaxRowsAsZero() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL)) {
+ pStmt.setMaxRows(0);
+ pStmt.execute();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ assertTrue(rowCount > 0);
+}
+ }
+
+ /**
+ * Test setting maxSize at a value lower than existing query limit
+ */
+ @Test
+ public void testSetMaxRowsLowerThanQueryLimit() throws SQLException {
+try (PreparedStatement pStmt =
connection.prepareStatement(SYS_OPTIONS_SQL_LIMIT_10)) {
+ int valueToSet = RANDOMIZER.nextInt(9)+1; //range:[1-9]
+ pStmt.setMaxRows(valueToSet);
+ pStmt.executeQuery();
+ ResultSet rs = pStmt.getResultSet();
+ int rowCount = 0;
+ while (rs.next()) {
+rs.getBytes(1);
+rowCount++;
+ }
+ logger.info("MaxRows was set as {} and query limited to 10 returned a
resultSet of {} rows", valueToSet, rowCount);
Review comment:
I had this when i was writing test cases. So, you're right... these are
redundant.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Implement JDBC Statement.setMaxRows() with System Option
>
>
> Key: DRILL-7048
> URL: https://issues.apache.org/jira/browse/DRILL-7048
> Project: Apache Drill
> Issue Type: New Feature
> Components: Client - JDBC, Query Planning & Optimization
>Affects Versions: 1.15.0
>Reporter: Kunal Khatua
>Assignee: Kunal Khatua
>Priority: Major
> Labels: doc-impacting
> Fix For: 1.17.0
>
>
> With DRILL-6960, the webUI will get an auto-limit on the number of results
> fetched.
> Since more of the plumbing is already there, it makes sense to provide the
> same for the JDBC client.
> In addition, it would be nice if the Server can have a pre-defined value as
> well (default 0; i.e. no limit) so that an _admin_ would be able to ensure a
> max limit on the resultset size as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (DRILL-7107) Unable to connect to Drill 1.15 through ZK
[ https://issues.apache.org/jira/browse/DRILL-7107?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sorabh Hamirwasia updated DRILL-7107: - Labels: ready-to-commit (was: ) > Unable to connect to Drill 1.15 through ZK > -- > > Key: DRILL-7107 > URL: https://issues.apache.org/jira/browse/DRILL-7107 > Project: Apache Drill > Issue Type: Bug >Reporter: Karthikeyan Manivannan >Assignee: Karthikeyan Manivannan >Priority: Major > Labels: ready-to-commit > Fix For: 1.16.0 > > > After upgrading to Drill 1.15, users are seeing they are no longer able to > connect to Drill using ZK quorum. They are getting the following "Unable to > setup ZK for client" error. > [~]$ sqlline -u "jdbc:drill:zk=172.16.2.165:5181;auth=maprsasl" > Error: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > (state=,code=0) > java.sql.SQLNonTransientConnectionException: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:174) > at > org.apache.drill.jdbc.impl.DrillJdbc41Factory.newDrillConnection(DrillJdbc41Factory.java:67) > at > org.apache.drill.jdbc.impl.DrillFactory.newConnection(DrillFactory.java:67) > at > org.apache.calcite.avatica.UnregisteredDriver.connect(UnregisteredDriver.java:138) > at org.apache.drill.jdbc.Driver.connect(Driver.java:72) > at sqlline.DatabaseConnection.connect(DatabaseConnection.java:130) > at sqlline.DatabaseConnection.getConnection(DatabaseConnection.java:179) > at sqlline.Commands.connect(Commands.java:1247) > at sqlline.Commands.connect(Commands.java:1139) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.lang.reflect.Method.invoke(Method.java:498) > at sqlline.ReflectiveCommandHandler.execute(ReflectiveCommandHandler.java:38) > at sqlline.SqlLine.dispatch(SqlLine.java:722) > at sqlline.SqlLine.initArgs(SqlLine.java:416) > at sqlline.SqlLine.begin(SqlLine.java:514) > at sqlline.SqlLine.start(SqlLine.java:264) > at sqlline.SqlLine.main(SqlLine.java:195) > Caused by: org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for > client. > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:340) > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:165) > ... 18 more > Caused by: java.lang.NullPointerException > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.findACLProvider(ZKACLProviderFactory.java:68) > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.getACLProvider(ZKACLProviderFactory.java:47) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:114) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:86) > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:337) > ... 19 more > Apache Drill 1.15.0.0 > "This isn't your grandfather's SQL." > sqlline> > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-7107) Unable to connect to Drill 1.15 through ZK
[
https://issues.apache.org/jira/browse/DRILL-7107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802261#comment-16802261
]
ASF GitHub Bot commented on DRILL-7107:
---
sohami commented on pull request #1702: DRILL-7107 Unable to connect to Drill
1.15 through ZK
URL: https://github.com/apache/drill/pull/1702#discussion_r269351700
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
##
@@ -81,23 +78,20 @@
private ConcurrentHashMap endpointsMap = new
ConcurrentHashMap();
private static final Pattern ZK_COMPLEX_STRING =
Pattern.compile("(^.*?)/(.*)/([^/]*)$");
- public ZKClusterCoordinator(DrillConfig config, String connect)
- throws IOException, DrillbitStartupException {
-this(config, connect, null);
+ public ZKClusterCoordinator(DrillConfig config, String connect) {
+this(config, connect, new DefaultACLProvider());
}
- public ZKClusterCoordinator(DrillConfig config, BootStrapContext context)
- throws IOException, DrillbitStartupException {
-this(config, null, context);
+ public ZKClusterCoordinator(DrillConfig config, ACLProvider aclProvider) {
+this(config, null, aclProvider);
Review comment:
Ok. Please open a JIRA to track it for future if needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Unable to connect to Drill 1.15 through ZK
> --
>
> Key: DRILL-7107
> URL: https://issues.apache.org/jira/browse/DRILL-7107
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Karthikeyan Manivannan
>Assignee: Karthikeyan Manivannan
>Priority: Major
> Fix For: 1.16.0
>
>
> After upgrading to Drill 1.15, users are seeing they are no longer able to
> connect to Drill using ZK quorum. They are getting the following "Unable to
> setup ZK for client" error.
> [~]$ sqlline -u "jdbc:drill:zk=172.16.2.165:5181;auth=maprsasl"
> Error: Failure in connecting to Drill:
> org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client.
> (state=,code=0)
> java.sql.SQLNonTransientConnectionException: Failure in connecting to Drill:
> org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client.
> at
> org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:174)
> at
> org.apache.drill.jdbc.impl.DrillJdbc41Factory.newDrillConnection(DrillJdbc41Factory.java:67)
> at
> org.apache.drill.jdbc.impl.DrillFactory.newConnection(DrillFactory.java:67)
> at
> org.apache.calcite.avatica.UnregisteredDriver.connect(UnregisteredDriver.java:138)
> at org.apache.drill.jdbc.Driver.connect(Driver.java:72)
> at sqlline.DatabaseConnection.connect(DatabaseConnection.java:130)
> at sqlline.DatabaseConnection.getConnection(DatabaseConnection.java:179)
> at sqlline.Commands.connect(Commands.java:1247)
> at sqlline.Commands.connect(Commands.java:1139)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at sqlline.ReflectiveCommandHandler.execute(ReflectiveCommandHandler.java:38)
> at sqlline.SqlLine.dispatch(SqlLine.java:722)
> at sqlline.SqlLine.initArgs(SqlLine.java:416)
> at sqlline.SqlLine.begin(SqlLine.java:514)
> at sqlline.SqlLine.start(SqlLine.java:264)
> at sqlline.SqlLine.main(SqlLine.java:195)
> Caused by: org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for
> client.
> at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:340)
> at
> org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:165)
> ... 18 more
> Caused by: java.lang.NullPointerException
> at
> org.apache.drill.exec.coord.zk.ZKACLProviderFactory.findACLProvider(ZKACLProviderFactory.java:68)
> at
> org.apache.drill.exec.coord.zk.ZKACLProviderFactory.getACLProvider(ZKACLProviderFactory.java:47)
> at
> org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:114)
> at
> org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:86)
> at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:337)
> ... 19 more
> Apache Drill 1.15.0.0
> "This isn't your grandfather's SQL."
> sqlline>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (DRILL-7107) Unable to connect to Drill 1.15 through ZK
[ https://issues.apache.org/jira/browse/DRILL-7107?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sorabh Hamirwasia updated DRILL-7107: - Affects Version/s: 1.15.0 > Unable to connect to Drill 1.15 through ZK > -- > > Key: DRILL-7107 > URL: https://issues.apache.org/jira/browse/DRILL-7107 > Project: Apache Drill > Issue Type: Bug > Components: Client - JDBC >Affects Versions: 1.15.0 >Reporter: Karthikeyan Manivannan >Assignee: Karthikeyan Manivannan >Priority: Major > Labels: ready-to-commit > Fix For: 1.16.0 > > > After upgrading to Drill 1.15, users are seeing they are no longer able to > connect to Drill using ZK quorum. They are getting the following "Unable to > setup ZK for client" error. > [~]$ sqlline -u "jdbc:drill:zk=172.16.2.165:5181;auth=maprsasl" > Error: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > (state=,code=0) > java.sql.SQLNonTransientConnectionException: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:174) > at > org.apache.drill.jdbc.impl.DrillJdbc41Factory.newDrillConnection(DrillJdbc41Factory.java:67) > at > org.apache.drill.jdbc.impl.DrillFactory.newConnection(DrillFactory.java:67) > at > org.apache.calcite.avatica.UnregisteredDriver.connect(UnregisteredDriver.java:138) > at org.apache.drill.jdbc.Driver.connect(Driver.java:72) > at sqlline.DatabaseConnection.connect(DatabaseConnection.java:130) > at sqlline.DatabaseConnection.getConnection(DatabaseConnection.java:179) > at sqlline.Commands.connect(Commands.java:1247) > at sqlline.Commands.connect(Commands.java:1139) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.lang.reflect.Method.invoke(Method.java:498) > at sqlline.ReflectiveCommandHandler.execute(ReflectiveCommandHandler.java:38) > at sqlline.SqlLine.dispatch(SqlLine.java:722) > at sqlline.SqlLine.initArgs(SqlLine.java:416) > at sqlline.SqlLine.begin(SqlLine.java:514) > at sqlline.SqlLine.start(SqlLine.java:264) > at sqlline.SqlLine.main(SqlLine.java:195) > Caused by: org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for > client. > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:340) > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:165) > ... 18 more > Caused by: java.lang.NullPointerException > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.findACLProvider(ZKACLProviderFactory.java:68) > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.getACLProvider(ZKACLProviderFactory.java:47) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:114) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:86) > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:337) > ... 19 more > Apache Drill 1.15.0.0 > "This isn't your grandfather's SQL." > sqlline> > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (DRILL-7107) Unable to connect to Drill 1.15 through ZK
[ https://issues.apache.org/jira/browse/DRILL-7107?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sorabh Hamirwasia updated DRILL-7107: - Component/s: Client - JDBC > Unable to connect to Drill 1.15 through ZK > -- > > Key: DRILL-7107 > URL: https://issues.apache.org/jira/browse/DRILL-7107 > Project: Apache Drill > Issue Type: Bug > Components: Client - JDBC >Reporter: Karthikeyan Manivannan >Assignee: Karthikeyan Manivannan >Priority: Major > Labels: ready-to-commit > Fix For: 1.16.0 > > > After upgrading to Drill 1.15, users are seeing they are no longer able to > connect to Drill using ZK quorum. They are getting the following "Unable to > setup ZK for client" error. > [~]$ sqlline -u "jdbc:drill:zk=172.16.2.165:5181;auth=maprsasl" > Error: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > (state=,code=0) > java.sql.SQLNonTransientConnectionException: Failure in connecting to Drill: > org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for client. > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:174) > at > org.apache.drill.jdbc.impl.DrillJdbc41Factory.newDrillConnection(DrillJdbc41Factory.java:67) > at > org.apache.drill.jdbc.impl.DrillFactory.newConnection(DrillFactory.java:67) > at > org.apache.calcite.avatica.UnregisteredDriver.connect(UnregisteredDriver.java:138) > at org.apache.drill.jdbc.Driver.connect(Driver.java:72) > at sqlline.DatabaseConnection.connect(DatabaseConnection.java:130) > at sqlline.DatabaseConnection.getConnection(DatabaseConnection.java:179) > at sqlline.Commands.connect(Commands.java:1247) > at sqlline.Commands.connect(Commands.java:1139) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.lang.reflect.Method.invoke(Method.java:498) > at sqlline.ReflectiveCommandHandler.execute(ReflectiveCommandHandler.java:38) > at sqlline.SqlLine.dispatch(SqlLine.java:722) > at sqlline.SqlLine.initArgs(SqlLine.java:416) > at sqlline.SqlLine.begin(SqlLine.java:514) > at sqlline.SqlLine.start(SqlLine.java:264) > at sqlline.SqlLine.main(SqlLine.java:195) > Caused by: org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for > client. > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:340) > at > org.apache.drill.jdbc.impl.DrillConnectionImpl.(DrillConnectionImpl.java:165) > ... 18 more > Caused by: java.lang.NullPointerException > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.findACLProvider(ZKACLProviderFactory.java:68) > at > org.apache.drill.exec.coord.zk.ZKACLProviderFactory.getACLProvider(ZKACLProviderFactory.java:47) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:114) > at > org.apache.drill.exec.coord.zk.ZKClusterCoordinator.(ZKClusterCoordinator.java:86) > at org.apache.drill.exec.client.DrillClient.connect(DrillClient.java:337) > ... 19 more > Apache Drill 1.15.0.0 > "This isn't your grandfather's SQL." > sqlline> > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
