> On May 15, 2014, 6:41 p.m., Aman Sinha wrote: > > exec/java-exec/src/main/codegen/data/AggrTypes1.tdd, line 92 > > <https://reviews.apache.org/r/21371/diff/3/?file=579603#file579603line92> > > > > It isn't clear why a separate class called Sum0 is needed ..is the > > existing Sum not sufficient ? If it is needed for Sum, what about other > > aggregate functions ?
It's only needed since Optiq converts a Sum to $Sum0, and I needed for the output type to be Optional (Nullable), so that the If Expression can both be optional types. I believe Optiq does it since it wants Sum0 to return 0 in no values case which we do already, but needed that function signature so Drill can understand. Any suggestions for alternatives how to address this? > On May 15, 2014, 6:41 p.m., Aman Sinha wrote: > > common/src/main/java/org/apache/drill/common/logical/data/Window.java, > > lines 107-108 > > <https://reviews.apache.org/r/21371/diff/3/?file=579599#file579599line107> > > > > Is 'Within' implying the OVER clause ? We should support empty OVER > > clause .. for example: SELECT SUM(a1) OVER (). Basically, no PARTITION BY > > or ORDER BY is specified, so I want to do the aggregate over entire data > > set. Within is actually the PARTITION by expressions, so PARTITION BY X with translate into Within [ NamedExpression("X") ] And what will the results looks like? For SUM it's just the same value for each row right? I'm a bit fuzzy what the Window function standards are, and have been referring to Optiq's Parser Test to see what valid options there are. > On May 15, 2014, 6:41 p.m., Aman Sinha wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java, > > line 205 > > <https://reviews.apache.org/r/21371/diff/3/?file=579626#file579626line205> > > > > Don't you need a next() interface in the WindowFrameRecordBatch ? If a > > downstream operator calls next() on the incoming, how does this class > > respond ? Note that normally we have the doWork() in the template class, > > not in the record batch class. I'm utilizing the AbstractSingleRecordBatch loop to do the right handling, as I found out I'm not doing much different handling than what the default does. If the downstream calls incoming it simply calls doWork again here. I've modified the interface so doWork can send different IterOutcome to downstream, where before it just sends OK all the time. > On May 15, 2014, 6:41 p.m., Aman Sinha wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java, > > line 79 > > <https://reviews.apache.org/r/21371/diff/3/?file=579641#file579641line79> > > > > It sounds like a single physical WindowPOP is being created for a list > > of logical Windows. Suppose there are 2 Window Functions: > > SUM(a1) OVER (PARTITION BY b1), > > SUM(a1) OVER (PARTITION BY c1) > > > > then, I believe Optiq would have created 2 logical window rels since > > the two Partition-By clauses are not compatible with each other. So, we > > will need two separate physical window operators, right ? > > > > On the other hand, suppose the 2 window functions are: > > RANK() OVER (ORDER BY a1, b1), > > RANK() OVER (ORDER BY a1) > > > > then we could do both in a single physical Window operator by doing > > ordering only once on (a1, b1). I believe what you said is true, let me modify it so it can output multiple ones. For the latter case the best way is to do add Optimizer rule to merge them then? - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21371/#review43130 ----------------------------------------------------------- On May 13, 2014, 9:01 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21371/ > ----------------------------------------------------------- > > (Updated May 13, 2014, 9:01 p.m.) > > > Review request for drill. > > > 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. > > This patch also addresses the CASE syntax bug (DRILL-665) > > Also fixed the Drill review patch tool. > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/expression/CastExpression.java > 7e5eea096b53ede676abb4764631fd92805b4ad1 > > common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java > 95888639f1f1de232c4df8b4baa38e7ca3d12ffb > common/src/main/java/org/apache/drill/common/expression/IfExpression.java > 280952dac4552b000ce05bc8922f8f82e979a4b0 > > common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java > 1efb029224c759caf7da0260704ad7756b1d521d > > common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java > 526275fc88d8c90a254ecbeb76343f37ec4f0695 > > common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java > bf67a6bd27f34d3dfb7e256b581238f0c4f531b6 > > common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java > ab94987811d6c988a55525ac8b8e041e11034f77 > > common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java > 799c9ddfed42e986e9ccc5235ef73ff13f7dfaff > > common/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java > e9bd03a7f9154b739847f9bf76510deefe847d82 > > 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/LogicalOperator.java > 531e6a6244b110a845833c5d94b2077df1f26460 > > 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 > 0099bb9a8a29f6c7e23ff7f45a80a1d714cb3ddd > > common/src/main/java/org/apache/drill/common/logical/data/visitors/LogicalVisitor.java > 4bf9fbfcaac21ab077ce26993575f679010bf592 > exec/java-exec/src/main/codegen/data/AggrTypes1.tdd > 71931dfbadde2f3981746e61d9ad40baf3c4890b > exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java > aa9aeab7af7d387cff9da57af69d3c9a5a9498fd > exec/java-exec/src/main/codegen/templates/TypeHelper.java > 4755e9210728967f17fbeddd2ef8b0d6c501d641 > > exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java > c0c8484005374ab273f5129e9f18afba0d2cb4f3 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java > d700bf3f045b21c8d15b0841f6568b39574c227d > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java > 0267be3ad54edf3e4da18582ffa8c83f7d8864fc > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java > 44210914d1b6f819716b366020d116c89e92587f > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java > e5c890eb81320ecb3affe67b74e900636296c480 > exec/java-exec/src/main/java/org/apache/drill/exec/memory/Accountor.java > 624042e03fb8f7fc980405597266044fea832e41 > exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java > 78c0e3cb9ec90de70fbf1e036d13f0853af4f8ad > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java > 01071012a9aebc5861dde6c4a2306efd3319cfbf > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java > 7a1440a4b2d7321efa086cc07f2a45468fa8a733 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Limit.java > b926e3ed7839fad6c107476f7c8ca9c2bbe0be1d > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java > 5bb378a8bed208e290b5c0d0437c066e0e5e1251 > > 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/filter/FilterRecordBatch.java > 566dfe0aaf8ea87584c6fcf14613ed745c805b6d > > 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 > ed56e796b84d713e697296017056e590af37169d > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java > b94f403731609ffb57574c32ffe299a0172ea91c > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java > 62af0b2a395d7b61ed334761f0e878f6affdc9d6 > > 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 > 6d720a7d1d3afba72109230f8642e8c7cc3b60f5 > > 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/DrillJoinRel.java > a5593e762256c89cd6659c9dff5b3d9171f5555b > > 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/DrillOptiq.java > 7efd7144153310f86b78e8b85c606f172b0ff672 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java > 0dd9b9ebba42f63db594e36564be47c2665870ee > > 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 > c07fee35d3895b6d7bfbbd30a108d58a9f6abb60 > > 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 > 7c8d7672cbc147fee793b0ed4d59cbfcd1136322 > > 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 > 772b3b9322b58f2cfdc84ee73c3717caac3fad4b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperator.java > b074ba0d5ae874ff663210f9fb873eb9b7f2a1cd > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > 747744026bc795d6ae6af2f98ee4af1937a8cf6e > > 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 > d107c29c0ef80426ec90ecd6e761a57cbf995455 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java > d6849f4cab0084a282945deb661b0cdc3f545d5b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java > 86ce6c56cfd0eda4f1e0f2e326eabd785c2643b2 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFileHandler.java > 4f7c424b72cbee48439370df681cb810c835c18b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java > 2a6dc6a33e679e1ff7f657727f2bc4628ef984e3 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java > 6905c1773ffbbe1a97d92201ee06aa9e5ac4ed39 > > 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 > 7be40cbe932efcf73586575ce611c4ce694308cb > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java > b362a20e149e7db5bf0687b970387f3dc160666c > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDescribeTable.java > 30c7e433bef7f82794200026de17a05fca0009cf > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java > bbb3a7f81f92c02410c4c879b357b02f3cf418af > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowFiles.java > f882ba9652528f357a2714952c18415d6eaa04ba > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowSchemas.java > 34695d950bc6b5e4b6369e91cc854f4580816d51 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlShowTables.java > 26d4fa216334d128ab92fba7dddd5868443a9424 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlUseSchema.java > 42a3914a7d103e4ccf04afd246bdc01e06aa683a > > exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java > dd2cfe059a191f223509f0b7cd8e8c6c8c097e89 > > exec/java-exec/src/main/java/org/apache/drill/exec/record/NullExpression.java > 15435504cd1aa5953c423192b6dc6b5c4794d1c5 > > exec/java-exec/src/main/java/org/apache/drill/exec/service/ServiceEngine.java > cfbde739511af4f96649479fe26677c05f86d064 > exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java > 7be8cc5f7498980a06e50dadd42105537fc4fa84 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFragmentRun.java > 842aa8fa66aff7c2a704f377af1fccca7534e167 > > 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/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 > be56b967f54ac1b27bd0ea4ae8a2015aecb99929 > exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java > 30a7144521117969db119791b0cb0896763c0494 > exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java > d087f7d6661a079cb37655cf67995de7f30f9a2a > tools/drill-patch-review.py c067ae2d805fa45707401953bb040f1f062ba920 > > Diff: https://reviews.apache.org/r/21371/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
