[jira] [Resolved] (DRILL-2937) Result for integer values from json files contains $numberLong in front of value
[ https://issues.apache.org/jira/browse/DRILL-2937?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Nadeau resolved DRILL-2937. --- Resolution: Invalid If you return complex data, we now use Extended Json encoding for complex values. If these fields are not complex values, or disabling extended json mode does not change the behavior, reopen the bug. Result for integer values from json files contains $numberLong in front of value -- Key: DRILL-2937 URL: https://issues.apache.org/jira/browse/DRILL-2937 Project: Apache Drill Issue Type: Bug Components: Client - C++ Reporter: Krystal Assignee: Parth Chandra Result of query from json files contains $numberLong appends to the front of integer values. Below is a sample result for such query: select t.c.`value` from (select flatten(x.fields.map) c from `dfs.drillTestDir`.`ta_netstat.snappy.parquet` x) t; { member1 : { $numberLong : 0 } } { member1 : { $numberLong : 0 } } { member0 : Searching } { member1 : { $numberLong : 0 } } { member0 : Home network registered } { member1 : { $numberLong : 0 } } { member3 : 0.0 } { member1 : { $numberLong : 2 } } { member0 : 17 } { member1 : { $numberLong : 5 } } { member1 : { $numberLong : 0 } } { member1 : { $numberLong : 1398440292 } } -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33779: DRILL-2932: Fix: Error text reported via System.out.println rather than thrown SQLException's message
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33779/#review82608 --- Ship it! Ship It! - Parth Chandra On May 5, 2015, 3:08 a.m., Daniel Barclay wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33779/ --- (Updated May 5, 2015, 3:08 a.m.) Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra. Bugs: DRILL-2932 https://issues.apache.org/jira/browse/DRILL-2932 Repository: drill-git Description --- DRILL-2932: Fix: Error reported via System.out rather than exception message Main: - Removed the System.out.println(...) from submissionFailed(...). - Put UserException's message text in SQLException's message (didn't just wrap). - Added undoing of extra wrapping by AvaticaStatement.execute...(...). - Created unit test for execute...(...) exceptions. - Refined related exception handling (handled cases separately, narrowed throws declarations and catches). JDBC cleanup: - Renamed ex - executionFailureException - Added getNext() method doc. comment. - Removed some obsolete alignment spaces. Added review this TODOs re DRILL-2933 (probably-obsolete SchemaChangeException): - DrillCursor; - MergingRecordBatch, ParquetResultListener, PrintingResultsListener, QueryWrapper, RecordBatchLoader, UnorderedReceiverBatch; - TestDrillbitResilience, TestTextColumn, BaseTestQuery, DrillTestWrapper. Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java 64e7266 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 09cb7ad exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java 088630c exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 3beae89 exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java aa43aa9 exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java d6e6d08 exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d05c896 exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java 2698701 exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java 0e80f91 exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java 3c1a38a exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java 41b89ce exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java ba265e6 exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 484a5e5 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java PRE-CREATION Diff: https://reviews.apache.org/r/33779/diff/ Testing --- Ran SQLLine to confirm change from System.out plus poor SQLException to good SQLException. Ran new specific unit test. Ran regular tests; no new failures. Thanks, Daniel Barclay
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
On May 5, 2015, 11:19 p.m., Jacques Nadeau wrote: Let's just have one exception OutOfMemoryException and make it a RuntimeException. I don't know what the purpose of having two different ones is. I meant to add: I don't think we need this to ever be a caught exception. - Jacques --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82606 --- On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 35bf3cd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 7b9fffb exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 74b7d85
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
On May 5, 2015, 11:19 p.m., Jacques Nadeau wrote: Let's just have one exception OutOfMemoryException and make it a RuntimeException. I don't know what the purpose of having two different ones is. Jacques Nadeau wrote: I meant to add: I don't think we need this to ever be a caught exception. Should I make this change as a separate patch after the new allocator is committed to master ? otherwise, I will introduce lot's of small changes that will cause this patch or the new allocator's patch too much rebasing overhead. - abdelhakim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82606 --- On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java
Re: Review Request 33859: DRILL-2951: Schema not getting specified when direct drillbit is specified in the connection URL
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33859/#review82618 --- Ship it! Ship It! - Hanifi Gunes On May 5, 2015, 7:11 p.m., Parth Chandra wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33859/ --- (Updated May 5, 2015, 7:11 p.m.) Review request for drill, Hanifi Gunes and Mehant Baid. Repository: drill-git Description --- DRILL-2951: Schema not getting specified when direct drillbit is specified in the connection URL Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 4576eb4be8de64f6099a0654b73ca1a48738121b Diff: https://reviews.apache.org/r/33859/diff/ Testing --- Connected to a specific drillbit and a specific schema defined. The ran show tables and queries against the tables. Note that if no schema is specified, the user has to choose a schema before issuing queries. Thanks, Parth Chandra
Re: Review Request 33770: DRILL-2697 Pause injections should pause indefinitely until signalled
On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java, line 58 https://reviews.apache.org/r/33770/diff/1/?file=947639#file947639line58 I'm not sure about this waiting uninterruptibly. We're going to start using Thread.interrupt() to regain control of threads that are blocked waiting on queues or sockets if the fragment they are running on behalf of is cancelled. Seems like this should be handled in the same way. I can talk to you about that more, and Venki can tell you about what he's doing in this area. This should not be a problem since this feature is used for testing (in a controlled environment). I feel it will in fact expose bugs; if not, we can remove this in the future. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java, line 142 https://reviews.apache.org/r/33770/diff/1/?file=947645#file947645line142 Does this mean that I can't have multiple pauses in the execution thread that will all work for a single query? For example, suppose I inject two pauses at different phases of execution: one just after planning and remote fragments are set up, and another after the first batch of results are returned. Will they both work, or will only the first one work? Yes, you can't. They will both be unpaused. I will re-submit a patch with client that unpauses a list of pause sites. - Sudheesh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/#review82573 --- On May 2, 2015, 1:09 a.m., Sudheesh Katkam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/ --- (Updated May 2, 2015, 1:09 a.m.) Review request for drill, Chris Westin and Jacques Nadeau. Repository: drill-git Description --- DRILL-2697: Pauses sites wait indefinitely for a resume signal DrillClient sends a resume signal to UserServer. UserServer triggers a resume call in the correct Foreman. Foreman resumes all pauses related to the query through the Control layer. + Fix for bug in FragmentExecutor#closeOutResources + Better error messages and more tests in TestDrillbitResilience and TestPauseInjection + Added execution controls to operator context + Removed ControlMessageHandler interface, renamed ControlHandlerImpl to ControlMessageHandler Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ae0f580 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControlRpcConfig.java 37730e3 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControlTunnel.java a4f9fdf exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserRpcConfig.java 88592d4 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java 9e929de exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java b7ffbf0 exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java 1171bf8 exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java e5f9c9c exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java a3ceb8f exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java b6c6852 exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlMessageHandler.java c5d78cc exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 34fa639 exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentManager.java 0ba91b4 exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java f526fbe exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java b1c3fe0 exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 8854ef3 exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java 2698701 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java 508b10c protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 protocol/src/main/java/org/apache/drill/exec/proto/UserProtos.java c072a47
Re: Review Request 33685: DRILL-2904: Fix wrong before rows message to after rows message
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33685/#review82611 --- Ship it! Ship It! - Parth Chandra On April 29, 2015, 8:50 p.m., Daniel Barclay wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33685/ --- (Updated April 29, 2015, 8:50 p.m.) Review request for drill, Mehant Baid and Parth Chandra. Bugs: DRILL-2904 https://issues.apache.org/jira/browse/DRILL-2904 Repository: drill-git Description --- - Worked around Avatica isBeforeFirst()/next() bug. - Fixed corresponding error in unit test. - Also fixed other backwards test-failure message wording. Diffs - exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java 2b20f23 exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java 64be408 Diff: https://reviews.apache.org/r/33685/diff/ Testing --- Manually checked (stepped through) unit test runs before and after fixing unit test. Ran regular tests; no new failures. Thanks, Daniel Barclay
[jira] [Created] (DRILL-2966) HAVING clause with CASE statement with IN predicate causes assertion
Aman Sinha created DRILL-2966: - Summary: HAVING clause with CASE statement with IN predicate causes assertion Key: DRILL-2966 URL: https://issues.apache.org/jira/browse/DRILL-2966 Project: Apache Drill Issue Type: Bug Components: Query Planning Optimization Affects Versions: 0.9.0 Reporter: Aman Sinha Assignee: Sean Hsuan-Yi Chu Fix For: 1.0.0 The following query fails in sql-to-rel conversion: {code} select count(*) from emp group by emp.deptno having sum(case when emp.empno in (1, 2, 3) then emp.sal else 0 end) between 1.0 and 2.0 {code} {code} java.lang.AssertionError: Internal error: while converting CASE WHEN `EMP`.`EMPNO` IN (1, 2, 3) THEN `EMP`.`SAL` ELSE 0 END at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4058) at org.apache.calcite.sql2rel.StandardConvertletTable.convertCase(StandardConvertletTable.java:301) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.apache.calcite.sql2rel.ReflectiveConvertletTable$1.convertCall(ReflectiveConvertletTable.java:87) at org.apache.calcite.sql2rel.SqlNodeToRexConverterImpl.convertCall(SqlNodeToRexConverterImpl.java:60) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4212) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:3668) at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:130) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4105) at org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4483) at org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4329) at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:130) at org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4528) at org.apache.calcite.sql2rel.SqlToRelConverter$AggConverter.visit(SqlToRelConverter.java:4329) at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:130) at org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2573) at org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2510) {code} This is dependent on CALCITE-694 . -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (DRILL-2967) Incompatible types error reported in a not in query with compatible data types
Victoria Markman created DRILL-2967: --- Summary: Incompatible types error reported in a not in query with compatible data types Key: DRILL-2967 URL: https://issues.apache.org/jira/browse/DRILL-2967 Project: Apache Drill Issue Type: Bug Components: Query Planning Optimization Affects Versions: 0.9.0 Reporter: Victoria Markman Assignee: Jinfeng Ni Two tables, parquet files (attached in the bug): {code} 0: jdbc:drill:schema=dfs select * from t1; ++++ | a1 | b1 | c1 | ++++ | 1 | a | 2015-01-01 | | 2 | b | 2015-01-02 | | 3 | c | 2015-01-03 | | 4 | null | 2015-01-04 | | 5 | e | 2015-01-05 | | 6 | f | 2015-01-06 | | 7 | g | 2015-01-07 | | null | h | 2015-01-08 | | 9 | i | null | | 10 | j | 2015-01-10 | ++++ 10 rows selected (0.119 seconds) 0: jdbc:drill:schema=dfs select * from t2; ++++ | a2 | b2 | c2 | ++++ | 0 | zzz| 2014-12-31 | | 1 | a | 2015-01-01 | | 2 | b | 2015-01-02 | | 2 | b | 2015-01-02 | | 2 | b | 2015-01-02 | | 3 | c | 2015-01-03 | | 4 | d | 2015-01-04 | | 5 | e | 2015-01-05 | | 6 | f | 2015-01-06 | | 7 | g | 2015-01-07 | | 7 | g | 2015-01-07 | | 8 | h | 2015-01-08 | | 9 | i | 2015-01-09 | ++++ 13 rows selected (0.116 seconds) {code} Disable hash join and set slice_target = 1: alter session set `planner.enable_hashjoin` = false; alter session set `planner.slice_target` = 1; Correct result: {code} 0: jdbc:drill:schema=dfs select * from t1 where b1 not in (select b2 from t2); ++++ | a1 | b1 | c1 | ++++ | 10 | j | 2015-01-10 | ++++ 1 row selected (0.625 seconds) {code} Swap tables and you get an error: {code} 0: jdbc:drill:schema=dfs select * from t2 where b2 not in (select b1 from t1); ++++ | a1 | b1 | c1 | ++++ Query failed: SYSTEM ERROR: Join only supports implicit casts between 1. Numeric data 2. Varchar, Varbinary data Left type: INT, Right type: VARCHAR. Add explicit casts to avoid this error Fragment 1:0 [1a83aa50-39aa-452c-91dd-970bf4a8f03d on atsqa4-133.qa.lab:31010] java.lang.RuntimeException: java.sql.SQLException: Failure while executing query. at sqlline.SqlLine$IncrementalRows.hasNext(SqlLine.java:2514) at sqlline.SqlLine$TableOutputFormat.print(SqlLine.java:2148) at sqlline.SqlLine.print(SqlLine.java:1809) at sqlline.SqlLine$Commands.execute(SqlLine.java:3766) at sqlline.SqlLine$Commands.sql(SqlLine.java:3663) at sqlline.SqlLine.dispatch(SqlLine.java:889) at sqlline.SqlLine.begin(SqlLine.java:763) at sqlline.SqlLine.start(SqlLine.java:498) at sqlline.SqlLine.main(SqlLine.java:460) {code} Explain plan for the query with an error: {code} 0: jdbc:drill:schema=dfs explain plan for select * from t2 where b2 not in (select b1 from t1); +++ |text|json| +++ | 00-00Screen 00-01 Project(*=[$0]) 00-02UnionExchange 01-01 Project(T27¦¦*=[$0]) 01-02SelectionVectorRemover 01-03 Filter(condition=[NOT(CASE(=($2, 0), false, IS NOT NULL($6), true, IS NULL($4), null, ($3, $2), null, false))]) 01-04MergeJoin(condition=[=($4, $5)], joinType=[left]) 01-06 SelectionVectorRemover 01-08Sort(sort0=[$4], dir0=[ASC]) 01-10 Project(T27¦¦*=[$0], b2=[$1], $f0=[$2], $f1=[$3], b20=[$4]) 01-12HashToRandomExchange(dist0=[[$4]]) 02-01 UnorderedMuxExchange 04-01Project(T27¦¦*=[$0], b2=[$1], $f0=[$2], $f1=[$3], b20=[$4], E_X_P_R_H_A_S_H_F_I_E_L_D=[castInt(hash64AsDouble($4))]) 04-02 Project(T27¦¦*=[$0], b2=[$1], $f0=[$2], $f1=[$3], b20=[$1]) 04-03NestedLoopJoin(condition=[true], joinType=[inner]) 04-05 Project(T27¦¦*=[$0], b2=[$1]) 04-06Scan(groupscan=[ParquetGroupScan [entries=[ReadEntryWithPath
Re: Review Request 33684: DRILL-2889: Rename JdbcTest to JdbcTestBase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33684/#review82613 --- Ship it! Ship It! - Parth Chandra On May 2, 2015, 5:31 a.m., Daniel Barclay wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33684/ --- (Updated May 2, 2015, 5:31 a.m.) Review request for drill, Hanifi Gunes and Parth Chandra. Bugs: DRILL-2889 https://issues.apache.org/jira/browse/DRILL-2889 Repository: drill-git Description --- Renamed `JdbcTest` to `JdbcTestBase` to follow `*Base` pattern of (some) test base classes and avoid confusion/conflicted with `*Test` pattern of _unit-level_ unit test classes).. Diffs - exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 0f3d838 exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java e486898 exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTestBase.java PRE-CREATION exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetGetMethodConversionsTest.java eced72b exec/jdbc/src/test/java/org/apache/drill/jdbc/SingleConnectionCachingFactory.java 7e3a51b exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2128GetColumnsDataTypeNotTypeCodeIntBugsTest.java 38c6d46 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2439GetBooleanFailsSayingWrongTypeBugTest.java bd993fd exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2461IntervalsBreakInfoSchemaBugTest.java d9ac3c7 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2463GetNullsFailedWithAssertionsBugTest.java 1d1361a exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2489CallsAfterCloseThrowExceptionsTest.java 3282945 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java f257c98 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcTestActionBase.java 15fe219 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcTestQueryBase.java 5c0a0e5 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestAggregateFunctionsQuery.java aa68e9f exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java a8f47c6 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 843c1c7 Diff: https://reviews.apache.org/r/33684/diff/ Testing --- Ran regular tests; no new failures. Thanks, Daniel Barclay
Re: Review Request 33683: DRILL-2887: Fix bad applications of JdbcTest.connect(...).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33683/#review82620 --- Ship it! Minor comment. Otherwise looks good. exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2128GetColumnsDataTypeNotTypeCodeIntBugsTest.java https://reviews.apache.org/r/33683/#comment133377 (minor) Why not use the same pattern as the other tests that use Driver.connect(...). - Parth Chandra On April 29, 2015, 8:50 p.m., Daniel Barclay wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33683/ --- (Updated April 29, 2015, 8:50 p.m.) Review request for drill, Hanifi Gunes and Parth Chandra. Bugs: DRILL-2887 https://issues.apache.org/jira/browse/DRILL-2887 Repository: drill-git Description --- - Removed use of Jdbc.connect() from: -- Drill2128GetColumnsDataTypeNotTypeCodeIntBugsTest -- Drill2489CallsAfterCloseThrowExceptionsTest - Added (manual) test for use of auto-resetting connection. [JdbcTest] - Documented need to avoid Jdbc.connect() in above and similar tests. - Add document TODO for JdbcTest. Diffs - exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java dd92c9b exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 0f3d838 exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java 8b885a4 exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java e486898 exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetGetMethodConversionsTest.java eced72b exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java 78a8af2 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2128GetColumnsDataTypeNotTypeCodeIntBugsTest.java 38c6d46 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2463GetNullsFailedWithAssertionsBugTest.java 1d1361a exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Drill2489CallsAfterCloseThrowExceptionsTest.java 3282945 Diff: https://reviews.apache.org/r/33683/diff/ Testing --- Ran regular tests; no new failures. Thanks, Daniel Barclay
[jira] [Created] (DRILL-2965) Unit tests for REST API
Sudheesh Katkam created DRILL-2965: -- Summary: Unit tests for REST API Key: DRILL-2965 URL: https://issues.apache.org/jira/browse/DRILL-2965 Project: Apache Drill Issue Type: Task Components: Client - HTTP Reporter: Sudheesh Katkam Assignee: Sudheesh Katkam Fix For: 1.2.0 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33840: DRILL-2423: Show proper message when trying to drop an unknown view.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33840/ --- (Updated May 5, 2015, 11:31 p.m.) Review request for drill and Jinfeng Ni. Changes --- Addressed review comments. Also added testcase for new code path. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2423 for details. Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 2428b45 exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestBaseViewSupport.java bfe113b exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 2ae6991 Diff: https://reviews.apache.org/r/33840/diff/ Testing --- Added unittests Thanks, Venki Korukanti
Re: TODOs in comments
My boss at Oracle used to say “There’s a TODO in this code. Why are you checking it in? It isn’t finished.” A bit harsh, but he had a good point. A TODO can indicate something that is not implemented because it is outside the current specification. If something isn’t supported functionality yet, throw new AssertionError(“cannot whizzbang yet”); seems better, in my opinion, than // TODO: fix this return null; Also, ask yourself who is likely to fix your TODO. Most likely it will be you. Or maybe no one will ever get round to it. Either way, you are cluttering up the code for everyone else who is not going to fix it. Lastly, there are genuinely cases where you wrote the simplest thing that could possibly work and you did not write a more efficient but more complex algorithm. Well done. This is good software engineering. You want to tell the world that your algorithm might be found to be slow, and you want to receive a little credit for having anticipated the problem. Also totally fine. But don’t write “// TODO: convert this bubble sort to a merge sort”. That makes it sounds as if there is a bug in your code, and your code is perfect (unless and until someone finds a performance issue). Just write a nice paragraph explaining your design choices, possible issues and how they would manifest themselves. I would not ban TODOs outright, but I think developers should think three times before checking one in. Julian On May 5, 2015, at 10:14 PM, Daniel Barclay dbarc...@maprtech.com wrote: I think that requiring every TODO note to have an associated JIRA report number would be too restrictive, increasing the friction to record notes about things to be looked at later. Making it so that one can't leave a note without the overhead of filing a whole JIRA report is going to cause some things to go unnoted. That seems significantly worse than not having every note indexed in JIRA. Encouraging having the JIRA number sounds fine; requiring it, at least without having some alternative mark for less-formal things, doesn't seem good. Daniel Sudheesh Katkam wrote: Yes, TODOs must have an associated JIRA, with the specified format. On May 5, 2015, at 1:14 PM, Julian Hyde julianh...@gmail.com wrote: Is the proposal to disallow TODOs that do not have a JIRA case number? I’d be +1 to that. I’m much less concerned with the problem that TODO(DRILL-abcd) might linger after in the code after DRILL-abcd has been fixed. Julian On May 5, 2015, at 12:38 PM, Jason Altekruse altekruseja...@gmail.com wrote: It could optionally be passed manually as a flag, but we already have the build step that is generating the git.properties file, we could issue the same type of call to git to try to pull the JIRA number out of the commit message. On Tue, May 5, 2015 at 12:34 PM, Chris Westin chriswesti...@gmail.com wrote: I like that idea, but how would the build know what JIRA you're working on? On Tue, May 5, 2015 at 12:25 PM, Jason Altekruse altekruseja...@gmail.com wrote: We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh -- Daniel Barclay MapR Technologies
[jira] [Resolved] (DRILL-1973) Tableau query causes parsing error
[ https://issues.apache.org/jira/browse/DRILL-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Nadeau resolved DRILL-1973. --- Resolution: Fixed Tableau query causes parsing error -- Key: DRILL-1973 URL: https://issues.apache.org/jira/browse/DRILL-1973 Project: Apache Drill Issue Type: Bug Components: Execution - Relational Operators Affects Versions: 0.7.0 Reporter: Chris Matta Assignee: Steven Phillips Fix For: 1.0.0 Attachments: 0001-DRILL-1973-Filter-should-check-for-record-count-befo.patch, information_schema.views.txt Query: {code:sql} SELECT * FROM ( SELECT SUM(1) AS `sum_Number_of_Records_ok` FROM `mfs.views`.`nestedclickview` `nestedclickview` INNER JOIN `mfs.views`.`productview` `productview` ON (`nestedclickview`.`prod_id` = `productview`.`prod_id`) HAVING (COUNT(1) 0) ) T LIMIT 0; {code} The views are from Andy P's Drill demo: https://github.com/andypern/drill-beta-demo.git productview is a view into a MapR-DB table, and nestedclickview is a view for nested JSON data. I get the following error on the foreman: {code} 2015-01-09 16:08:43,042 [2b5002f4-cb62-1832-6331-ef89dbe2314b:foreman] ERROR o.a.drill.exec.work.foreman.Foreman - Error e0 295344-2881-4a81-b739-0e20026b21b1: Query failed: Failure parsing SQL. Encountered ) at line 1, column 24. {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (DRILL-2959) Compression codecs are leaking or slow to recapture memory
Jacques Nadeau created DRILL-2959: - Summary: Compression codecs are leaking or slow to recapture memory Key: DRILL-2959 URL: https://issues.apache.org/jira/browse/DRILL-2959 Project: Apache Drill Issue Type: Bug Components: Storage - Other Reporter: Jacques Nadeau Assignee: Jacques Nadeau Fix For: 1.0.0 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (DRILL-2958) Move Drill to alternative cost-based planner for Join planning
Jacques Nadeau created DRILL-2958: - Summary: Move Drill to alternative cost-based planner for Join planning Key: DRILL-2958 URL: https://issues.apache.org/jira/browse/DRILL-2958 Project: Apache Drill Issue Type: Bug Components: Query Planning Optimization Reporter: Jacques Nadeau Assignee: Jinfeng Ni Fix For: 1.0.0 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 32273: DRILL-2408: Invalid (0 length) parquet file created by CTAS
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32273/ --- (Updated May 5, 2015, 3:40 p.m.) Review request for drill, Aman Sinha, Jason Altekruse, and Venki Korukanti. Changes --- fixed how BaseTestQuery updates the storage plugin to use one single temp folder path for all drillbits rebased on top of master Bugs: DRILL-2408 https://issues.apache.org/jira/browse/DRILL-2408 Repository: drill-git Description --- I updated ParquetRecordWriter to delete the last file created if it's empty (no records written to it). I also added two unit tests one that checks the default case where we try to create a table using a query that returns 0 rows, the second case can happen if the ParquetRecordWriter flushes it's content just after writing the last record, it will then create a new empty file. Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3506ffa exec/java-exec/src/main/java/org/apache/drill/exec/util/TestUtilities.java a1fcc2a exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java d6e6d08 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 5670e1e exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriterEmptyFiles.java PRE-CREATION exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java c73eb50 Diff: https://reviews.apache.org/r/32273/diff/ Testing --- - Added 2 unit tests to TestParquetWriter - Unit tests are passing Thanks, abdelhakim deneche
[jira] [Resolved] (DRILL-1866) Tests that include limit sporadically fail when run as part of entire test suite on Linux
[ https://issues.apache.org/jira/browse/DRILL-1866?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Nadeau resolved DRILL-1866. --- Resolution: Fixed Tests that include limit sporadically fail when run as part of entire test suite on Linux - Key: DRILL-1866 URL: https://issues.apache.org/jira/browse/DRILL-1866 Project: Apache Drill Issue Type: Bug Components: Tools, Build Test Reporter: Jacques Nadeau Assignee: Steven Phillips Priority: Critical Fix For: 1.0.0 Seems to be a timing issue where memory is not being released as part of limit cancellation of a query. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Resolved] (DRILL-2093) Columns of time and timestamp data type are not stored correctly in json format on CTAS
[ https://issues.apache.org/jira/browse/DRILL-2093?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Nadeau resolved DRILL-2093. --- Resolution: Fixed Columns of time and timestamp data type are not stored correctly in json format on CTAS --- Key: DRILL-2093 URL: https://issues.apache.org/jira/browse/DRILL-2093 Project: Apache Drill Issue Type: Bug Components: Storage - JSON Affects Versions: 0.8.0 Reporter: Victoria Markman Assignee: Jacques Nadeau Priority: Critical Fix For: 1.0.0 Attachments: t1.csv I have a csv file and am trying to create matching file in json format. {code} alter session set `store.format` = 'json'; create table test_json(c_varchar, c_integer, c_bigint, c_smalldecimal, c_bigdecimal, c_float, c_date, c_time, c_timestamp, c_boolean) as select case when columns[0] = '' then cast(null as varchar(255)) else cast(columns[0] as varchar(255)) end, case when columns[1] = '' then cast(null as integer) else cast(columns[1] as integer) end, case when columns[2] = '' then cast(null as bigint) else cast(columns[2] as bigint) end, case when columns[3] = '' then cast(null as decimal(18,4)) else cast(columns[3] as decimal(18, 4)) end, case when columns[4] = '' then cast(null as decimal(38,4)) else cast(columns[4] as decimal(38, 4)) end, case when columns[5] = '' then cast(null as float) else cast(columns[5] as float) end, case when columns[6] = '' then cast(null as date) else cast(columns[6] as date) end, case when columns[7] = '' then cast(null as time) else cast(columns[7] as time) end, case when columns[8] = '' then cast(null as timestamp) else cast(columns[8] as timestamp) end, case when columns[9] = '' then cast(null as boolean) else cast(columns[9] as boolean) end from `t1.csv`; {code} Create table succeeds, but I can't read back time or timestamp: {code} 0: jdbc:drill:schema=dfs select cast(c_time as time) from test_json; Query failed: RemoteRpcException: Failure while running fragment., Invalid format: 1970-01-01T08:13:16.000Z is malformed at 70-01-01T08:13:16.000Z [ b3f6c0c9-01e4-410f-919d-a899ede35ed9 on atsqa4-133.qa.lab:31010 ] [ b3f6c0c9-01e4-410f-919d-a899ede35ed9 on atsqa4-133.qa.lab:31010 ] Error: exception while executing query: Failure while executing query. (state=,code=0) 0: jdbc:drill:schema=dfs select c_time from test_json; ++ | c_time | ++ | 1970-01-01T08:13:16.000Z | | 1970-01-01T04:15:45.000Z | | 1970-01-01T18:21:06.000Z | | 1970-01-01T13:35:54.000Z | | 1970-01-01T05:17:11.000Z | ++ 5 rows selected (0.055 seconds) 0: jdbc:drill:schema=dfs select cast(c_timestamp as timestamp) from test_json; Query failed: RemoteRpcException: Failure while running fragment., Invalid format: 2014-03-16T03:55:21.000Z is malformed at T03:55:21.000Z [ 527a42e5-2eb7-41d7-a55b-c6cee0779db6 on atsqa4-133.qa.lab:31010 ] [ 527a42e5-2eb7-41d7-a55b-c6cee0779db6 on atsqa4-133.qa.lab:31010 ] Error: exception while executing query: Failure while executing query. (state=,code=0) {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (DRILL-2957) Netty Memory Manager doesn't move empty chunks between lists
Jacques Nadeau created DRILL-2957: - Summary: Netty Memory Manager doesn't move empty chunks between lists Key: DRILL-2957 URL: https://issues.apache.org/jira/browse/DRILL-2957 Project: Apache Drill Issue Type: Bug Components: Execution - Flow Reporter: Jacques Nadeau Assignee: Hanifi Gunes Priority: Critical Fix For: 1.0.0 I'm seeing a pattern in the memory allocator and I need you to take a look at it. Here are the basic concepts: 1) We use an extension of PooledByteBufAllocator [1] called PooledByteBufAllocatorL. 2) We use many Direct Arenas (generally one per core) 3) Each arena has chunk lists for different occupancies (chunks that are empty, chunks 25% full, chunks 50% full, etc) [2] 4) Each of these chunk lists maintains a list of chunks. The chunks move from list to list as they get more or less full. 5) When no memory is being used, chunks move back to the empty list. 6) If there are excessive empty chunks, they are released back to the OS. (I don't remember the exact trigger here and I'm only seeing this sometimes right now.) We're running on Netty 4.0.27. What I'm seeing is that we don't seem to be moving the chunks back to the empty list as they are vacated. You can see an example output from my memory logging [3] that is enabled by [4]. I haven't replicated this at small scale but at large scale I see it consistently (30 node cluster, large group by query [5]). I want to understand this behavior better, determine if it is a bug or not and determine whether or not this hurts memory for subsequent queries. One other note, Netty will cache small amounts of memory that is allocated and released on the same thread for that thread. I don't believe this is a large amount of memory but be aware of it. It should be possible to control this using these settings [6]. [1] https://github.com/netty/netty/blob/master/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java [2] https://github.com/netty/netty/blob/master/buffer/src/main/java/io/netty/buffer/PoolArena.java#L67 [3] Memory log output at idle after large query (one example arena out of 32 on perf cluster, see logs on those nodes for more info): ::snip:: Chunk(s) at 0~25%: none Chunk(s) at 0~50%: Chunk(62194b16: 0%, 0/16777216) Chunk(35983868: 1%, 8192/16777216) Chunk(5bbfb16a: 1%, 163840/16777216) Chunk(1c6d277e: 1%, 8192/16777216) Chunk(2897b6bf: 2%, 204800/16777216) Chunk(287d5c71: 0%, 0/16777216) Chunk(s) at 25~75%: Chunk(61bad0ee: 0%, 0/16777216) Chunk(s) at 50~100%: Chunk(2d79a032: 0%, 0/16777216) Chunk(42415f4e: 0%, 0/16777216) Chunk(33a3bade: 0%, 0/16777216) Chunk(1ce7ca63: 0%, 0/16777216) Chunk(531e1888: 0%, 0/16777216) Chunk(54786a09: 0%, 0/16777216) Chunk(5cdcb359: 0%, 0/16777216) Chunk(3e40137b: 0%, 0/16777216) Chunk(534f0fb3: 0%, 0/16777216) Chunk(6301ee8a: 0%, 0/16777216) Chunk(6a90c3aa: 0%, 0/16777216) Chunk(s) at 75~100%: none Chunk(s) at 100%: none ::snip:: [4] Enable the memory logger by enabling trace level debugging for the drill.allocator logger like this: logger name=drill.allocator additivity=false level value=trace / appender-ref ref=FILE / appender-ref ref=SOCKET / /logger [5] On perf cluster # sqllineTPCDS ALTER SESSION SET `exec.errors.verbose` = true; ALTER SESSION SET `planner.enable_multiphase_agg` = false; ALTER SESSION SET `store.parquet.block-size` = 134217728; ALTER SESSION SET `planner.enable_mux_exchange` = false; ALTER SESSION SET `exec.min_hash_table_size` = 67108864; ALTER SESSION SET `planner.enable_hashagg` = true; ALTER SESSION SET `planner.memory.max_query_memory_per_node` = 29205777612; ALTER SESSION SET `planner.width.max_per_node` = 23; create table dfs.tmp.agg33 as select ss_sold_date_sk , ss_sold_time_sk , ss_item_sk , ss_customer_sk , ss_cdemo_sk, count(*) from `store_sales_dri3` group by ss_sold_date_sk , ss_sold_time_sk , ss_item_sk , ss_customer_sk , ss_cdemo_sk; [6] https://github.com/netty/netty/blob/master/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L98 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33833: DRILL-2848: Part 2: Provide option to disable decimal type
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33833/#review82592 --- I only looked the changes related to planner setting /rule, and leave the parquet related change to Jason. The planner part change looks good to me. - Jinfeng Ni On May 4, 2015, 6:11 p.m., Mehant Baid wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33833/ --- (Updated May 4, 2015, 6:11 p.m.) Review request for drill, Jason Altekruse and Jinfeng Ni. Repository: drill-git Description --- This patch adds an option to enable/ disable decimal data type. Disabled casting to decimal, reading decimal from parquet and hive. Diffs - contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBasePushFilterIntoScan.java f1f3a0b contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 8c400ea contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoPushDownFilterForScan.java 4fd80bd exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java 92e5678 exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java 441f2e3 exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillParseContext.java be4474f exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/PruneScanRule.java c8be019 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/FilterPrel.java b631cdc exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/FlattenPrel.java e206951 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java 8f089c4 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/ProjectAllowDupPrel.java cc215f8 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/ProjectPrel.java 35fa5be exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java c918723 exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 1636a25 exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 33b2a4c exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java 0cf12b4 exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetReaderUtility.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java 11d0042 exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetToDrillTypeConverter.java 7c3eeb8 exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java 389c1f6 exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 921d134 exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetRecordMaterializer.java 574df40 exec/java-exec/src/main/java/org/apache/drill/exec/work/ExecErrorConstants.java PRE-CREATION exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java c627ff2 exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java 504524d exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java 3abd193 exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java 67131c1 exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastEmptyStrings.java 3e05c0e exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java 2c23df4 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 5670e1e exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java 5991046 exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/columnreaders/TestColumnReaderFactory.java 9ae6b78 exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet2/TestDrillParquetReader.java 782191f exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcTestQueryBase.java 5c0a0e5 exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestAggregateFunctionsQuery.java aa68e9f Diff: https://reviews.apache.org/r/33833/diff/ Testing --- Added negative tests. Modified existing unit tests to use the newly added parameter. Thanks, Mehant Baid
Re: Review Request 33770: DRILL-2697 Pause injections should pause indefinitely until signalled
On May 5, 2015, 9:31 p.m., Chris Westin wrote: A few minor things, along with some questions about resumeAll and the implications of that. I may have more suggestions based on the answers to my questions. Some of those might be best answered in the form of comments in the code or class javadoc because it seems likely others will have the same questions later. I have some comments already but I will added a detailed explanation in PauseInjection class. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line 183 https://reviews.apache.org/r/33770/diff/1/?file=947643#file947643line183 Should this wait on acceptExternalEvents, in the same way cancel() does? What happens if we're not completely set up and we execute these? The queryContext one seems like it might be ok, but the queryManager one definitely seems like it could be a problem if this comes in before all the remote fragments are set up. Right before the foreman-ready pause site, Foreman checks to make sure that if resume was called before it is completely setup that it calls resume again. The method is safe to call anytime and multiple times. If resume was called before Foreman is setup, the queryManager does not signal any fragments because there are no fragments in fragmentDataSet. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line 184 https://reviews.apache.org/r/33770/diff/1/?file=947643#file947643line184 All of them? I thought we were going to control this by queryId? Execution controls lives within FragmentContext and QueryContext of a specific query; so this resume is for all pauses within current query context. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line 264 https://reviews.apache.org/r/33770/diff/1/?file=947643#file947643line264 Should you clear the resume flag after this? No, the resume flag is to check if resume was called before Foreman is setup. It can be set to true once and remains true (since all pauses are resumed). On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java, line 404 https://reviews.apache.org/r/33770/diff/1/?file=947650#file947650line404 I couldn't see why you need to use a Pointer here instead of just the Exception. If an exception happens in the CancellingThread, the test does not fail. This ensures such an exception is caught. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java, line 633 https://reviews.apache.org/r/33770/diff/1/?file=947650#file947650line633 Why not use assertTrue() here? If the assert fails, the exception happens in the RPC thread and the test just hangs. The check method has a workaround for that to make the test fail. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java, line 660 https://reviews.apache.org/r/33770/diff/1/?file=947650#file947650line660 assertTrue()? ^ same as above. On May 5, 2015, 9:31 p.m., Chris Westin wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentManager.java, line 52 https://reviews.apache.org/r/33770/diff/1/?file=947646#file947646line52 How about some javadoc explaining what this means? From this it's not at all obvious that this is about injected pauses instead of something like suspend/resume. Would unpause() be a better name? I think unpause is a better name. - Sudheesh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/#review82573 --- On May 2, 2015, 1:09 a.m., Sudheesh Katkam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/ --- (Updated May 2, 2015, 1:09 a.m.) Review request for drill, Chris Westin and Jacques Nadeau. Repository: drill-git Description --- DRILL-2697: Pauses sites wait indefinitely for a resume signal DrillClient sends a resume signal to UserServer. UserServer triggers a resume call in the correct Foreman. Foreman resumes all pauses related to the query through the Control layer. + Fix for bug in FragmentExecutor#closeOutResources + Better error messages and more tests in TestDrillbitResilience and TestPauseInjection + Added execution controls to operator context + Removed
[jira] [Resolved] (DRILL-2617) Errors in the execution stack will cause DeferredException to throw an IllegalStateException
[ https://issues.apache.org/jira/browse/DRILL-2617?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Deneche A. Hakim resolved DRILL-2617. - Resolution: Fixed DeferredException.close() is no longer called by FragmentExecutor. The only class that calls DeferredException.close() is EventProcessor but the exception is only used as a local variable and once the exception is closed, no other method is called. Errors in the execution stack will cause DeferredException to throw an IllegalStateException Key: DRILL-2617 URL: https://issues.apache.org/jira/browse/DRILL-2617 Project: Apache Drill Issue Type: Bug Components: Execution - Flow Affects Versions: 0.8.0 Reporter: Deneche A. Hakim Assignee: Deneche A. Hakim Fix For: 1.0.0 When a query fails while executing, the following events happen: - the exception is added to {{FragmentContext.deferredException}} - the {{FragmentExecutor}} reports the failure to the client through the {{Foreman}} - the {{FragmentExecutor}} closes the {{DeferredException}} - {{DeferredException.close()}} throws back the original exception - {{FragmentExecutor.run()}} catches the exception and try to add it to the {{DeferredException}} - {{DeferredException.addException()}} throws an {{IllegalStateException}} because it's already closed. You can reproduce this by querying the following json file, which contains an extra : {code} { a1: 0 , b1: a} { a1: 1 , b1: b} { a1: 2 , b1: c} { a1:: 3 , b1: c} {code} Sqlline will dispaly both the error message sent by the Foreman and the IllegalStateException: {noformat} 0: jdbc:drill:zk=local select * from `t.json`; Query failed: Query stopped., Unexpected character (':' (code 58)): expected a valid value (number, String, array, object, 'true', 'false' or 'null') at [Source: org.apache.drill.exec.vector.complex.fn.JsonReader@161188d3; line: 3, column: 9] [ b55f7d53-0e88-456f-bb12-160cacae9222 on administorsmbp2.attlocal.net:31010 ] Error: exception while executing query: Failure while executing query. (state=,code=0) 0: jdbc:drill:zk=local Exception in thread WorkManager-2 java.lang.IllegalStateException at com.google.common.base.Preconditions.checkState(Preconditions.java:133) at org.apache.drill.common.DeferredException.addException(DeferredException.java:47) at org.apache.drill.common.DeferredException.addThrowable(DeferredException.java:61) at org.apache.drill.exec.ops.FragmentContext.fail(FragmentContext.java:135) at org.apache.drill.exec.work.fragment.FragmentExecutor.run(FragmentExecutor.java:181) at org.apache.drill.common.SelfCleaningRunnable.run(SelfCleaningRunnable.java:38) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745) {noformat} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33839: DRILL-2598: Throw validation error if CREATE VIEW/CTAS contains duplicate columns in definition
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33839/ --- (Updated May 5, 2015, 4:19 p.m.) Review request for drill and Jinfeng Ni. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2598 for details. Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 50af972 exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java PRE-CREATION exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 2ae6991 Diff: https://reviews.apache.org/r/33839/diff/ Testing --- Added several unittests to cover various scenarios in both CREATE VIEW and CTAS. Thanks, Venki Korukanti
[jira] [Created] (DRILL-2960) Default hive storage plugin missing from fresh drill install
Krystal created DRILL-2960: -- Summary: Default hive storage plugin missing from fresh drill install Key: DRILL-2960 URL: https://issues.apache.org/jira/browse/DRILL-2960 Project: Apache Drill Issue Type: Bug Components: Storage - Hive Affects Versions: 0.9.0 Reporter: Krystal Assignee: Venki Korukanti Installed drill on a fresh node. The default storage plugin for hive is missing from the webUI. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
TODOs in comments
Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Review Request 33859: DRILL-2951: Schema not getting specified when direct drillbit is specified in the connection URL
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33859/ --- Review request for drill, Hanifi Gunes and Mehant Baid. Repository: drill-git Description --- DRILL-2951: Schema not getting specified when direct drillbit is specified in the connection URL Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 4576eb4be8de64f6099a0654b73ca1a48738121b Diff: https://reviews.apache.org/r/33859/diff/ Testing --- Connected to a specific drillbit and a specific schema defined. The ran show tables and queries against the tables. Note that if no schema is specified, the user has to choose a schema before issuing queries. Thanks, Parth Chandra
[jira] [Created] (DRILL-2962) Correlates subquery with scalar aggregate or scalar aggregate with expression throws and error
Victoria Markman created DRILL-2962: --- Summary: Correlates subquery with scalar aggregate or scalar aggregate with expression throws and error Key: DRILL-2962 URL: https://issues.apache.org/jira/browse/DRILL-2962 Project: Apache Drill Issue Type: Bug Components: Query Planning Optimization Affects Versions: 0.9.0 Reporter: Victoria Markman Assignee: Aman Sinha Priority: Critical Fix For: 1.0.0 Filing this bug on Aman's request. It is not the same as DRILL-2949 DRILL-2949 has two outer tables. {code} 0: jdbc:drill:schema=dfs select * from t1 where a1 (select avg(a2)*100 from t2 where c1 = c2); Query failed: SYSTEM ERROR: Non-scalar sub-query used in an expression See Apache Drill JIRA: DRILL-1937 [991f5544-d44f-4ba2-aa70-783a88a5b16a on atsqa4-133.qa.lab:31010] Error: exception while executing query: Failure while executing query. (state=,code=0) Correlated scalar aggregation without expression: 0: jdbc:drill:schema=dfs select * from t1 where a1 (select avg(a2) from t2 where c1 = c2); Query failed: SYSTEM ERROR: This query cannot be planned possibly due to either a cartesian join or an inequality join [f238c0c0-efea-4684-8ec3-899fbcc2a075 on atsqa4-133.qa.lab:31010] Error: exception while executing query: Failure while executing query. (state=,code=0) {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: TODOs in comments
We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Re: TODOs in comments
I like that idea, but how would the build know what JIRA you're working on? On Tue, May 5, 2015 at 12:25 PM, Jason Altekruse altekruseja...@gmail.com wrote: We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Re: TODOs in comments
It could optionally be passed manually as a flag, but we already have the build step that is generating the git.properties file, we could issue the same type of call to git to try to pull the JIRA number out of the commit message. On Tue, May 5, 2015 at 12:34 PM, Chris Westin chriswesti...@gmail.com wrote: I like that idea, but how would the build know what JIRA you're working on? On Tue, May 5, 2015 at 12:25 PM, Jason Altekruse altekruseja...@gmail.com wrote: We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Re: Review Request 33840: DRILL-2423: Show proper message when trying to drop an unknown view.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33840/ --- (Updated May 5, 2015, 7:21 a.m.) Review request for drill and Jinfeng Ni. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2423 for details. Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 2428b45 exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestBaseViewSupport.java bfe113b exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 2ae6991 Diff: https://reviews.apache.org/r/33840/diff/ Testing --- Added unittests Thanks, Venki Korukanti
Re: TODOs in comments
Is the proposal to disallow TODOs that do not have a JIRA case number? I’d be +1 to that. I’m much less concerned with the problem that TODO(DRILL-abcd) might linger after in the code after DRILL-abcd has been fixed. Julian On May 5, 2015, at 12:38 PM, Jason Altekruse altekruseja...@gmail.com wrote: It could optionally be passed manually as a flag, but we already have the build step that is generating the git.properties file, we could issue the same type of call to git to try to pull the JIRA number out of the commit message. On Tue, May 5, 2015 at 12:34 PM, Chris Westin chriswesti...@gmail.com wrote: I like that idea, but how would the build know what JIRA you're working on? On Tue, May 5, 2015 at 12:25 PM, Jason Altekruse altekruseja...@gmail.com wrote: We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82579 --- Ship it! Ship It! - Steven Phillips On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 35bf3cd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 7b9fffb exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 74b7d85 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java aa9297e exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java af45815
[jira] [Created] (DRILL-2963) Exists with empty left batch causes IllegalStateException
Mehant Baid created DRILL-2963: -- Summary: Exists with empty left batch causes IllegalStateException Key: DRILL-2963 URL: https://issues.apache.org/jira/browse/DRILL-2963 Project: Apache Drill Issue Type: Bug Reporter: Mehant Baid Assignee: Mehant Baid Fix For: 1.0.0 In NestedLoopJoinBatch we don't correctly handle the case when we have an empty left input batch, need to return NONE in that case. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
On May 5, 2015, 8:24 p.m., Steven Phillips wrote: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line 210 https://reviews.apache.org/r/33829/diff/1/?file=949299#file949299line210 Should we also catch OutOfMemoryRuntimeException? Yes - abdelhakim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82574 --- On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 35bf3cd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 7b9fffb exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 74b7d85
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82574 --- exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java https://reviews.apache.org/r/33829/#comment133308 Does this exception need to be part of the signature? Is it just for documentation purposes? exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java https://reviews.apache.org/r/33829/#comment133306 Should we also catch OutOfMemoryRuntimeException? - Steven Phillips On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 35bf3cd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 7b9fffb
Re: TODOs in comments
Yes, TODOs must have an associated JIRA, with the specified format. On May 5, 2015, at 1:14 PM, Julian Hyde julianh...@gmail.com wrote: Is the proposal to disallow TODOs that do not have a JIRA case number? I’d be +1 to that. I’m much less concerned with the problem that TODO(DRILL-abcd) might linger after in the code after DRILL-abcd has been fixed. Julian On May 5, 2015, at 12:38 PM, Jason Altekruse altekruseja...@gmail.com wrote: It could optionally be passed manually as a flag, but we already have the build step that is generating the git.properties file, we could issue the same type of call to git to try to pull the JIRA number out of the commit message. On Tue, May 5, 2015 at 12:34 PM, Chris Westin chriswesti...@gmail.com wrote: I like that idea, but how would the build know what JIRA you're working on? On Tue, May 5, 2015 at 12:25 PM, Jason Altekruse altekruseja...@gmail.com wrote: We should also consider adding something to the build that will automate the process of checking for todo comments containing the JIRA number being fixed. This would allow reviewers to easily verify that a JIRA being closed is not leaving related TODOs in the code (possibly unit tests added by the reporter of the issue, or comments mentioned in another patch that wanted to relate a problem they saw in a known outstanding JIRA). This can be mitigated by mentioning the relevant areas in the code when filing a JIRA, but this would also be a helpful safety net to keep the code cleaner. - Jason On Tue, May 5, 2015 at 12:07 PM, Sudheesh Katkam skat...@maprtech.com wrote: Hey y’all, For consistency across code, Chris had recommended using TODO(DRILL-) format for todos in comments, where is the JIRA number. If there are no objections, let’s try to stick to that format. Thank you, Sudheesh
Re: Review Request 33829: DRILL-2757: Verify operators correctly handle low memory conditions and cancellations
On May 5, 2015, 8:24 p.m., Steven Phillips wrote: exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java, line 53 https://reviews.apache.org/r/33829/diff/1/?file=949297#file949297line53 Does this exception need to be part of the signature? Is it just for documentation purposes? abdelhakim deneche wrote: It's just for documentation purposes. I will remove it from the signature but leave it in the javadoc - abdelhakim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/#review82574 --- On May 5, 2015, 2:51 a.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33829/ --- (Updated May 5, 2015, 2:51 a.m.) Review request for drill, Hanifi Gunes, Jacques Nadeau, and Steven Phillips. Bugs: DRILL-2757 https://issues.apache.org/jira/browse/DRILL-2757 Repository: drill-git Description --- includes: [DRILL-2893](https://issues.apache.org/jira/browse/DRILL-2893): ScanBatch throws a NullPointerException instead of returning OUT_OF_MEMORY [DRILL-2894](https://issues.apache.org/jira/browse/DRILL-2894): FixedValueVectors shouldn't set it's data buffer to null when it fails to allocate it [DRILL-2895](https://issues.apache.org/jira/browse/DRILL-2895): AbstractRecordBatch.buildSchema() should properly handle OUT_OF_MEMORY outcome [DRILL-2905](https://issues.apache.org/jira/browse/DRILL-2905): RootExec implementations should properly handle IterOutcome.OUT_OF_MEMORY [DRILL-2920](https://issues.apache.org/jira/browse/DRILL-2920): properly handle OutOfMemoryException [DRILL-2947](https://issues.apache.org/jira/browse/DRILL-2947): AllocationHelper.allocateNew() doesn't have a consistent behavior when it can't allocate also: - improved how system errors are displayed - added UserException.memoryError() with a pre assigned error message - injection site in ScanBatch and unit test that runs various tpch queries and injects an exception in the ScanBatch that will cause an OUT_OF_MEMORY outcome to be sent Diffs - common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java 4da4ee8 common/src/main/java/org/apache/drill/common/exceptions/UserException.java 9283339 common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java a145f95 exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 6a924b7 exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 8a4b663 exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java 4700dbd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 5b4d7bd exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 67062f3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 9f6bea9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java 15fb7b5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b753574 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 1b90dd8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java c1c5cb9 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 86f3100 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d2282c8 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java 26f2657 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java dd53477 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6466f70 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java d20bfa1 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java eff9e61 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java c36b0d3 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java cf7ba16 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 35bf3cd
Re: Review Request 33662: DRILL-2902: Add support for context functions: user (synonyms session_user and system_user) and current_schema
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33662/ --- (Updated May 5, 2015, 6:45 p.m.) Review request for drill and Mehant Baid. Changes --- addressed review comment. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2902 for details. Apart from adding new UDFs, also refactored the context information stored in PlanFragment into a separate message. Refactored QueryDateTimeInfo into ContextInformation to provide one interface for all query context replated info. Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 4576eb4 exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ContextFunctions.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateTypeFunctions.java 9c932d6 exec/java-exec/src/main/java/org/apache/drill/exec/ops/ContextInformation.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 6414f56 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java f3cc666 exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java 1cdece1 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java 66ba229 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java 527bac0 exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java 8efb9e7 exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 4249cbe exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestContextFunctions.java PRE-CREATION exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 04e1980 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java 9758eb0 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 6a6a7e0 exec/java-exec/src/test/java/org/apache/drill/exec/pop/TestFragmentChecker.java 32e3bf9 exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestCustomUserAuthenticator.java 70d43b6 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java 604f375 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java 508b10c protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 protocol/src/main/java/org/apache/drill/exec/proto/SchemaBitControl.java 5e7562e protocol/src/main/java/org/apache/drill/exec/proto/beans/PlanFragment.java f6fbce1 protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryContextInformation.java PRE-CREATION protocol/src/main/protobuf/BitControl.proto 0424725 Diff: https://reviews.apache.org/r/33662/diff/ Testing --- Added unittests to test the new UDFs added in the patch. Thanks, Venki Korukanti
Re: Review Request 33662: DRILL-2902: Add support for context functions: user (synonyms session_user and system_user) and current_schema
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33662/#review82541 --- Ship it! Ship It! - Mehant Baid On May 4, 2015, 10:09 p.m., Venki Korukanti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33662/ --- (Updated May 4, 2015, 10:09 p.m.) Review request for drill and Mehant Baid. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2902 for details. Apart from adding new UDFs, also refactored the context information stored in PlanFragment into a separate message. Refactored QueryDateTimeInfo into ContextInformation to provide one interface for all query context replated info. Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 4576eb4 exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ContextFunctions.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateTypeFunctions.java 9c932d6 exec/java-exec/src/main/java/org/apache/drill/exec/ops/ContextInformation.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 6414f56 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java f3cc666 exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java 1cdece1 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java 66ba229 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java 527bac0 exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java 8efb9e7 exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 4249cbe exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestContextFunctions.java PRE-CREATION exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 04e1980 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java 9758eb0 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 6a6a7e0 exec/java-exec/src/test/java/org/apache/drill/exec/pop/TestFragmentChecker.java 32e3bf9 exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestCustomUserAuthenticator.java 70d43b6 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java 604f375 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java 508b10c protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 protocol/src/main/java/org/apache/drill/exec/proto/SchemaBitControl.java 5e7562e protocol/src/main/java/org/apache/drill/exec/proto/beans/PlanFragment.java f6fbce1 protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryContextInformation.java PRE-CREATION protocol/src/main/protobuf/BitControl.proto 0424725 Diff: https://reviews.apache.org/r/33662/diff/ Testing --- Added unittests to test the new UDFs added in the patch. Thanks, Venki Korukanti
Re: Review Request 33662: DRILL-2902: Add support for context functions: user (synonyms session_user and system_user) and current_schema
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33662/#review82537 --- exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ContextFunctions.java https://reviews.apache.org/r/33662/#comment133277 Should we check if the allocated buffer has enough space? - Mehant Baid On May 4, 2015, 10:09 p.m., Venki Korukanti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33662/ --- (Updated May 4, 2015, 10:09 p.m.) Review request for drill and Mehant Baid. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2902 for details. Apart from adding new UDFs, also refactored the context information stored in PlanFragment into a separate message. Refactored QueryDateTimeInfo into ContextInformation to provide one interface for all query context replated info. Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 4576eb4 exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ContextFunctions.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateTypeFunctions.java 9c932d6 exec/java-exec/src/main/java/org/apache/drill/exec/ops/ContextInformation.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 09a7568 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 6414f56 exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java f3cc666 exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java 1cdece1 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java 66ba229 exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java 527bac0 exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java 8efb9e7 exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 4249cbe exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestContextFunctions.java PRE-CREATION exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 04e1980 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java 9758eb0 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 6a6a7e0 exec/java-exec/src/test/java/org/apache/drill/exec/pop/TestFragmentChecker.java 32e3bf9 exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestCustomUserAuthenticator.java 70d43b6 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java 604f375 exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java 508b10c protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 protocol/src/main/java/org/apache/drill/exec/proto/SchemaBitControl.java 5e7562e protocol/src/main/java/org/apache/drill/exec/proto/beans/PlanFragment.java f6fbce1 protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryContextInformation.java PRE-CREATION protocol/src/main/protobuf/BitControl.proto 0424725 Diff: https://reviews.apache.org/r/33662/diff/ Testing --- Added unittests to test the new UDFs added in the patch. Thanks, Venki Korukanti
Re: Review Request 32273: DRILL-2408: Invalid (0 length) parquet file created by CTAS
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32273/#review82553 --- Ship it! Ship It! - Venki Korukanti On May 5, 2015, 3:40 p.m., abdelhakim deneche wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32273/ --- (Updated May 5, 2015, 3:40 p.m.) Review request for drill, Aman Sinha, Jason Altekruse, and Venki Korukanti. Bugs: DRILL-2408 https://issues.apache.org/jira/browse/DRILL-2408 Repository: drill-git Description --- I updated ParquetRecordWriter to delete the last file created if it's empty (no records written to it). I also added two unit tests one that checks the default case where we try to create a table using a query that returns 0 rows, the second case can happen if the ParquetRecordWriter flushes it's content just after writing the last record, it will then create a new empty file. Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3506ffa exec/java-exec/src/main/java/org/apache/drill/exec/util/TestUtilities.java a1fcc2a exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java d6e6d08 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 5670e1e exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriterEmptyFiles.java PRE-CREATION exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java c73eb50 Diff: https://reviews.apache.org/r/32273/diff/ Testing --- - Added 2 unit tests to TestParquetWriter - Unit tests are passing Thanks, abdelhakim deneche
[jira] [Created] (DRILL-2961) Statement.setQueryTimeout() should throw a SQLException
Kunal Khatua created DRILL-2961: --- Summary: Statement.setQueryTimeout() should throw a SQLException Key: DRILL-2961 URL: https://issues.apache.org/jira/browse/DRILL-2961 Project: Apache Drill Issue Type: Bug Components: Client - JDBC Affects Versions: 1.0.0 Environment: RHEL 6.4 Reporter: Kunal Khatua Assignee: Daniel Barclay (Drill) Fix For: 1.0.0 When trying to set the timeout for a Drill Statement object, Drill does not report any SQLException which makes the developer incorrectly believe that a timeout has been set. The operation should throw the exception: java.sql.SQLException: Method not supported at org.apache.drill.jdbc.DrillStatement.setQueryTimeout(DrillStatement.java) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 33840: DRILL-2423: Show proper message when trying to drop an unknown view.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33840/#review82587 --- Ship it! Looks good to me. Just have 2 minor comments. exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java https://reviews.apache.org/r/33840/#comment10 Why do we report Views don't exist in this schema if schema is not mutable? exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java https://reviews.apache.org/r/33840/#comment11 Will it better that we report different error for the case of view not exists (existingTable == null) and for the case of there exists other type of entry with the identical name (existingTable.getJdbcTableType() != Schema.TableType.VIEW) ? - Jinfeng Ni On May 5, 2015, 12:21 a.m., Venki Korukanti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33840/ --- (Updated May 5, 2015, 12:21 a.m.) Review request for drill and Jinfeng Ni. Repository: drill-git Description --- Please see https://issues.apache.org/jira/browse/DRILL-2423 for details. Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 2428b45 exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestBaseViewSupport.java bfe113b exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 2ae6991 Diff: https://reviews.apache.org/r/33840/diff/ Testing --- Added unittests Thanks, Venki Korukanti
Re: Review Request 33770: DRILL-2697 Pause injections should pause indefinitely until signalled
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/#review82573 --- A few minor things, along with some questions about resumeAll and the implications of that. I may have more suggestions based on the answers to my questions. Some of those might be best answered in the form of comments in the code or class javadoc because it seems likely others will have the same questions later. exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java https://reviews.apache.org/r/33770/#comment133305 I'm not sure about this waiting uninterruptibly. We're going to start using Thread.interrupt() to regain control of threads that are blocked waiting on queues or sockets if the fragment they are running on behalf of is cancelled. Seems like this should be handled in the same way. I can talk to you about that more, and Venki can tell you about what he's doing in this area. exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java https://reviews.apache.org/r/33770/#comment133310 Should this wait on acceptExternalEvents, in the same way cancel() does? What happens if we're not completely set up and we execute these? The queryContext one seems like it might be ok, but the queryManager one definitely seems like it could be a problem if this comes in before all the remote fragments are set up. exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java https://reviews.apache.org/r/33770/#comment133307 All of them? I thought we were going to control this by queryId? exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java https://reviews.apache.org/r/33770/#comment133309 Should you clear the resume flag after this? exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java https://reviews.apache.org/r/33770/#comment133316 Does this mean that I can't have multiple pauses in the execution thread that will all work for a single query? For example, suppose I inject two pauses at different phases of execution: one just after planning and remote fragments are set up, and another after the first batch of results are returned. Will they both work, or will only the first one work? exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentManager.java https://reviews.apache.org/r/33770/#comment133319 How about some javadoc explaining what this means? From this it's not at all obvious that this is about injected pauses instead of something like suspend/resume. Would unpause() be a better name? exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java https://reviews.apache.org/r/33770/#comment133322 I couldn't see why you need to use a Pointer here instead of just the Exception. exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java https://reviews.apache.org/r/33770/#comment133323 Instead of TC 1, TC 2, etc, can you please just copy-paste the one-line descriptions of these test cases from the original. Much easier if it's all described here inline. exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java https://reviews.apache.org/r/33770/#comment133324 Why not use assertTrue() here? exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java https://reviews.apache.org/r/33770/#comment133325 assertTrue()? exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java https://reviews.apache.org/r/33770/#comment133327 break the line at .setUserName() so only one attribute is set per line. protocol/src/main/protobuf/BitControl.proto https://reviews.apache.org/r/33770/#comment133329 Can we clean up this whitespace, and leave just a newline? - Chris Westin On May 1, 2015, 6:09 p.m., Sudheesh Katkam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/ --- (Updated May 1, 2015, 6:09 p.m.) Review request for drill, Chris Westin and Jacques Nadeau. Repository: drill-git Description --- DRILL-2697: Pauses sites wait indefinitely for a resume signal DrillClient sends a resume signal to UserServer. UserServer triggers a resume call in the correct Foreman. Foreman resumes all pauses related to the query through the Control layer. + Fix for bug in FragmentExecutor#closeOutResources + Better error messages and more tests in TestDrillbitResilience and TestPauseInjection + Added execution controls to operator context + Removed ControlMessageHandler interface, renamed ControlHandlerImpl to ControlMessageHandler