drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdelhakim deneche" <adene...@gmail.com>
Subject Re: Review Request 32468: DRILL-2498: Separate QueryResult into two messages QueryResult and QueryData
Date Thu, 02 Apr 2015 02:25:00 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32468/
-----------------------------------------------------------

(Updated April 2, 2015, 2:24 a.m.)


Review request for drill, Chris Westin, Daniel Barclay, and Jacques Nadeau.


Changes
-------

addressed Chris and Daniel comments


Bugs: DRILL-2498
    https://issues.apache.org/jira/browse/DRILL-2498


Repository: drill-git


Description
-------

INITIAL PATCH (looking for feedback)

- removed **last chunk** from *QueryResult*
- separate **QueryResult** and **QueryData**, update RPC layer to handle this separation
  . **QueryResultHandler** handles data and result messages separately
  . *UserClient* calls *QueryResultHandler.resultArrived()* / *dataArrived()* depending on
the received message
  . bumped up **RPC_VERSION** in *UserRpcConfig*
  . *UserServer* has separate *sendData()* / *sendResult()*
- updated **UserResultHandler** interface to handle data messages differently from result
messages:
  . *dataArrived(QueryResultBatch result, ConnectionThrottle throttle)* is called when a data
batch is received
  . *queryCompleted()* is called when a terminal (completed/canceled) message is received
- the *Foreman* only sends QueryResult to the client
- In case of an error, The *ScreenRoot* doesn't send the error to the client. The Foreman
will send a FAILED *QueryResult* instead
- in case of a data batch, the *ScreenRoot* sends a *QueryData* to the client
- *QueryWritableBatch* and *QueryResultBatch* use *QueryData* instead of *QueryResult*
- *VectorRecordMaterializer* builds a *QueryData* instead of *QueryResult*
- updated classes that implement *UserResultHandlers*:
  . DrillClient.ListHoldingResultsListener
  . PrintingResultsListener
  . QueryWrapper.Listener
  . BaseTestQuery.SilentListener
  . SingleRowListener
  . ParquetResultListener
  . TestParquetPhysicalPlan
  . DrillResultSet.ResultsListener
- updated several unit tests that were expecting an extra **last chunk** batch
  . TestDateFunctions
  . TestMultiInputAdd
  . TestCastVarCharToBigInt
  . TestDateTypes
- contains **DRILL-2521** (revert from proto 2.6.0 to 2.5.0)
  will be removed from the final patch
- code cleanup: making loggers private, removing unused imports


Diffs (updated)
-----

  contrib/storage-hbase/src/test/java/org/apache/drill/hbase/BaseHBaseTest.java b955d3b 
  contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseCFAsJSONString.java
6fe1525 
  contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestHiveUDFs.java
e134aac 
  contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestSampleHiveUDFs.java
9ef766f 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 6d4c86c 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java 926e703

  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 404c453

  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/materialize/QueryWritableBatch.java
2a59e22 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/materialize/RecordMaterializer.java
221fc34 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/materialize/VectorRecordMaterializer.java
cc1b3bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryDataBatch.java PRE-CREATION

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultBatch.java ab4c9ef

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java c05b127

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 925154d 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 9f83a4f

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserRpcConfig.java 908d304 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java c76d324 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java 8996a69

  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 285b75a 
  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 64cf2ec 
  exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java 75a91b3 
  exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java 80b4d13 
  exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java 3d19229 
  exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java 07cb833 
  exec/java-exec/src/test/java/org/apache/drill/exec/TestQueriesOnLargeFile.java 67b102d 
  exec/java-exec/src/test/java/org/apache/drill/exec/client/DrillClientSystemTest.java 98919ec

  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunction.java 3a744fd

  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java 67aa4dd

  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMultiInputAdd.java 65848eb

  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewAggregateFunctions.java
0238932 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestBroadcastExchange.java
5212125 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java
0d9f014 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastVarCharToBigInt.java
84ac8cf 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
fc4c0cc 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestDecimal.java 800f172

  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestDistributedFragmentRun.java
3de9c63 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestExtractFunctions.java
12c1c03 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHashToRandomExchange.java
10ee46a 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 6bf23ec

  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java
2880b18 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFragmentRun.java
bd15309 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestUnionExchange.java
271af72 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java
5a897c6 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java
6c21a28 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoin.java
d105272 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java
996b675 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/mergereceiver/TestMergingReceiver.java
d61c123 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/orderedpartitioner/TestOrderedPartitionExchange.java
27d38e6 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java
669a21f 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java
530883b 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
a96c507 
  exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestDateTypes.java 4cc82e4

  exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java 9bc0552

  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
9999be0 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
52d5086 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetPhysicalPlan.java
6cb412c 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java e5a2c94

  exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TextRecordReaderTest.java
fa43d55 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/fn/TestJsonReaderWithSparseFiles.java
437bbb5 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestComplexToJson.java
eedbf6f 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
fe86192 
  exec/java-exec/src/test/java/org/apache/drill/exec/work/batch/TestSpoolingBuffer.java b3c653f

  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java cddd999 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 0ce33f4 
  protocol/pom.xml f7d907d 
  protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java 67beed1 
  protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 2502701 
  protocol/src/main/java/org/apache/drill/exec/proto/BitData.java b4a2f19 
  protocol/src/main/java/org/apache/drill/exec/proto/CoordinationProtos.java 45321ea 
  protocol/src/main/java/org/apache/drill/exec/proto/ExecProtos.java ca4f294 
  protocol/src/main/java/org/apache/drill/exec/proto/GeneralRPCProtos.java 32a63e5 
  protocol/src/main/java/org/apache/drill/exec/proto/SchemaDefProtos.java 024774a 
  protocol/src/main/java/org/apache/drill/exec/proto/SchemaUserBitShared.java 68e86db 
  protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java 00a4905 
  protocol/src/main/java/org/apache/drill/exec/proto/UserProtos.java 330e774 
  protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryData.java PRE-CREATION 
  protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryResult.java d8eb92a 
  protocol/src/main/java/org/apache/drill/exec/proto/beans/RpcType.java 3f1f9fd 
  protocol/src/main/protobuf/User.proto 6c41a37 
  protocol/src/main/protobuf/UserBitShared.proto 1971e62 

Diff: https://reviews.apache.org/r/32468/diff/


Testing
-------

Unit tests are passing. Can't test functional/customer/tpch100


Thanks,

abdelhakim deneche


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message