> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > common/src/main/java/org/apache/drill/common/logical/data/Limit.java, line 
> > 66
> > <https://reviews.apache.org/r/24778/diff/1/?file=662190#file662190line66>
> >
> >     It isn't clear why you removed the node builder.  Can you explain?

It wasn't used that's why I removed it. Let me reset it for this patch.


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > common/src/main/java/org/apache/drill/common/expression/IfExpression.java, 
> > line 59
> > <https://reviews.apache.org/r/24778/diff/1/?file=662184#file662184line59>
> >
> >     Can you separate out your patch for all your cleanups that have nothing 
> > to do with the window function?

I'll try to :)


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java, line 105
> > <https://reviews.apache.org/r/24778/diff/1/?file=662200#file662200line105>
> >
> >     Same as above (sum0)

This is actually needed for window function. Optiq automatically inserts sum0 
for the window version of sum, so I need to able to handle it.


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java,
> >  line 111
> > <https://reviews.apache.org/r/24778/diff/1/?file=662216#file662216line111>
> >
> >     Same as above.

This is intended with the window change though, still want series of patches?


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java,
> >  line 56
> > <https://reviews.apache.org/r/24778/diff/1/?file=662222#file662222line56>
> >
> >     Can you call this Streaming[name]

You mean the RecordBatch or the framer?


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java,
> >  line 122
> > <https://reviews.apache.org/r/24778/diff/1/?file=662222#file662222line122>
> >
> >     This looks wrong.  Why do you need to match certain field names?  I 
> > think this is a dangerous assumption that will cause problems depending on 
> > what planning is applied after Optiq generates initial field names.  We 
> > should be able to get everything we need from the POP?

I only want to keep the optiq inserted window expressions, and didn't see a 
obvious way to retrieve only the window expressions. I tried selecting as 
w0$... before and seems like Optiq doesn't allow that. You have any suggestions?


> On Sept. 22, 2014, 1:05 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestUtils.java,
> >  line 39
> > <https://reviews.apache.org/r/24778/diff/1/?file=662261#file662261line39>
> >
> >     We're trying to deprecate SimpleRootExec.  Is there a way you can 
> > similar tests using Jason's new testing validation framework?  I don't 
> > really want to add new tests using SimpleRootExec.

Where is Jason's new testing validation framework?


- Timothy


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


On Aug. 17, 2014, 12:45 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24778/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2014, 12:45 a.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Bugs: DRILL-705
>     https://issues.apache.org/jira/browse/DRILL-705
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently only supports partitioning/ordering, not yet preceding or after 
> offsets
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/expression/CastExpression.java 
> 7e5eea096b53ede676abb4764631fd92805b4ad1 
>   
> common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java
>  0e778c5ebfba827a88826cd169d5e18e3f68440f 
>   common/src/main/java/org/apache/drill/common/expression/IfExpression.java 
> f16d8fe0951681ca138025abf86b0f5d25b1a6e6 
>   common/src/main/java/org/apache/drill/common/expression/NullExpression.java 
> f515d1467475b04dade0b878b3121e35a6520bc4 
>   
> common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
>  4c75459e37b65623541c2556f2cc6eda64912c96 
>   
> common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
>  a2a0d7fd279004dd8526728991a779f7b92c558f 
>   
> common/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java
>  69cb4eb73ab71c6424752a1027ee190ee95104c4 
>   
> common/src/main/java/org/apache/drill/common/logical/data/AbstractSingleBuilder.java
>  e733fddc554f462dadcc96e21db29191000f6ea1 
>   common/src/main/java/org/apache/drill/common/logical/data/Limit.java 
> 110204be8bac10b960013d8cc52d552c50f15995 
>   
> common/src/main/java/org/apache/drill/common/logical/data/LogicalOperatorBase.java
>  69a1c3c5e7e9911fc6fe1cccb16f8e6fb50dc293 
>   
> common/src/main/java/org/apache/drill/common/logical/data/NamedExpression.java
>  4c006c609e281a03607101fb83ae368bd4e43200 
>   
> common/src/main/java/org/apache/drill/common/logical/data/SingleInputOperator.java
>  0a5015c15a1e9a9b6c3b092ca644ec425d142957 
>   common/src/main/java/org/apache/drill/common/logical/data/Window.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/logical/data/WindowFrame.java 
> 09524061601209bd0d322fabade3df4ebdcd07bd 
>   
> common/src/main/java/org/apache/drill/common/logical/data/visitors/AbstractLogicalVisitor.java
>  87584449f2b0dfddc5ede5c766b79095bcc95603 
>   
> common/src/main/java/org/apache/drill/common/logical/data/visitors/LogicalVisitor.java
>  4bf9fbfcaac21ab077ce26993575f679010bf592 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
>  c4813a75b25b8001e1b5543ae523a24ae12a4aef 
>   exec/java-exec/src/main/codegen/data/AggrTypes1.tdd 
> 1bac07ee30558018932117d9e0d978b75742df83 
>   exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java 
> aa9aeab7af7d387cff9da57af69d3c9a5a9498fd 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java 
> 9bb25075730876996f02c5249f300cdda35dd882 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
>  c65951de52296a8051d4c3b0a542d7fb9841cd2b 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
>  caf91793b08730596d4b2305ed8e18d1ad210890 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/Accountor.java 
> cd0836749b941d0e01f0b48db7651372fc7f7bb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java 
> 24d9cfe528c4ef7d930f216a7428557d29ffe357 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java
>  5e854251bc0c97efd584edfa2e1f91861b8d1077 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractSingle.java
>  6c0b98f8650d21d743b95ca0ef6e05cde8524662 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalOperator.java
>  8f51390293fea89c278ae280bcf8bd5f8bb64a2a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java
>  f0b0b9a84642bc6bf74184be8b0fae58b9270da4 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Limit.java 
> 0038e4e0aae9af926f1c26f7186b2bd711a5bc18 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/InternalBatch.java
>  3e6def128034ccfce2dab38cef1ccff61553a104 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
>  39131125dfb08e1db11051049b33dbcd62620404 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java
>  3ece98beb6c6aec4898000a8ca158aaebbb630ed 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java
>  ccbf755faa8f1332df68e34bee130c7a27f503e0 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java
>  12ee40617fc3cdf82cb58f313c49f52cf2964ff2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java
>  0fcf4f358966a0cddf70c32ebbdd5cb7d031f30a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
>  1ba0103473b6d12981705a3abc16c2a84b2dbdaf 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java
>  42f21283687538a00eea6afbc69a2044cbbf711c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java
>  b012cec1d8a4d81ab6508de76147562fc2f9f75e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameTemplate.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFramer.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillWindowRelBase.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java
>  3fc3b89d1d7dbed0e890b6f942064851c9d09353 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java
>  fe5130ced1acfbe548b3af9b42d0cb80b7840faa 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillLimitRule.java
>  c3b0d00c6bddf52cb88307a49df2574b720a4806 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java
>  829eb1406a68c447c2cc319e36c847174d7ea195 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRel.java
>  7eca54e9b9a79f63d0b9df1bebc7400f46996028 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  63de69c95942661929513aae95aa7c23f2abb255 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillWindowRel.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillWindowRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitPrel.java
>  a4bb5035519f59dd746ada6efdf9200ce86f51e3 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
>  e34d3d15a4bfc5a5d0674b626215d702345a7896 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperator.java
>  299712ef9afa0900a9966b21da060fcd80ad9842 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
>  321d79d4e1631077d06daeee7cc66595fdb3d987 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  b7d9bd7f3e70a8754f9ae6a56986c4ea7f99e432 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
>  177331d9ac2b6bab4e62206e2985c042b51a97e9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java
>  e6f1fe1c4b8e7e0c4edb55fd5d669dda6274e7ab 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java
>  63db153ba4c8850be3c77433bc9696be085a5c01 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFileHandler.java
>  17e80bd3ec09e47896a19d5e6fa24db874b8c01b 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java
>  5e77628f022a1cec74196c6bb99a565204de0959 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java
>  a1c5aee92ed90018b02ba28e772ba5cb3fb6ccc6 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/DrillSqlCall.java
>  5fa592a13c0e8e12f04121e2302339d48189ad63 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateTable.java
>  01707354631ec371e7f1b819acf45ef265379809 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java
>  3666b4cccfd0608921febf96e1e60e67c95a6737 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDescribeTable.java
>  15da822586f5a42f4ae4b66dc229d3793b0194b2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java
>  1d943901ab99ba3b14346a9fe850153c7f7332e8 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowFiles.java
>  690e5fd8c33eecf6dda6692e22c54b82843760c1 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowSchemas.java
>  faa69d10700985f88c03ac7443f3c2e0b523431e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowTables.java
>  e66199a6c6c4eee05a3f1dd378ab3a69b2e7997f 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlUseSchema.java
>  76c7df3a5a7786f5b96c233787a820f22ebbcc76 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java
>  bea7bbfe9243e6d8d0bf415a336b9fe02dce7dad 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
>  49c739916426d727e1c088b2a6aa4fb5d9cc2c2e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/schema/Field.java 
> 14049efaa7b2684e3f537985ec42977bb1863657 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java
>  48d14662e02c6371283419d42094600dcd23b6a5 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 
> e7c6dc06fc9c90a90fecdd9c2757f580d4be5fc8 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestUtils.java
>  PRE-CREATION 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestSimpleLimit.java
>  0d5d62216b43c104f74d6cf50bd255e3db3e54c2 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
>  PRE-CREATION 
>   exec/java-exec/src/test/resources/window/mediumData.json PRE-CREATION 
>   exec/java-exec/src/test/resources/window/oneKeyCount.json PRE-CREATION 
>   exec/java-exec/src/test/resources/window/oneKeyCountData.json PRE-CREATION 
>   exec/java-exec/src/test/resources/window/oneKeyCountMultiBatch.json 
> PRE-CREATION 
>   exec/java-exec/src/test/resources/window/twoKeys.json PRE-CREATION 
>   exec/java-exec/src/test/resources/window/twoKeysData.json PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 
> bde0d3fac684ad1202b3566a4c688ce8de9d3b43 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java 
> 39ba043c08fdcedfbeac4b0327e2bd7372d41596 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 
> b681719a8f93059c2b4f16abc112d5178048c44c 
>   protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java 
> d5b235245e7938fe31330f201c9f3fc7290a06f9 
>   
> protocol/src/main/java/org/apache/drill/exec/proto/beans/CoreOperatorType.java
>  0c83e06800e7323b5d0271b2ba39d211cf1530a9 
>   protocol/src/main/protobuf/UserBitShared.proto 
> 10dce1fb9095193b01671d0ef05ee26a85fb416a 
>   tools/drill-patch-review.py c067ae2d805fa45707401953bb040f1f062ba920 
> 
> Diff: https://reviews.apache.org/r/24778/diff/
> 
> 
> Testing
> -------
> 
> Added few unit tests around window.
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Reply via email to