Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1226
Thanks for making the changes. +1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1216
+1
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1216#discussion_r183097212
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/planner/logical/TestTransitiveClosure.java
---
@@ -0,0 +1,102 @@
+/*
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1226#discussion_r183087404
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
---
@@ -238,26 +238,27 @@ protected DrillRel
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1226#discussion_r183091126
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java ---
@@ -178,6 +178,12 @@ public RuleSet getRules
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1223#discussion_r182903822
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestImpl.java
---
@@ -0,0 +1,137 @@
+/*
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1223#discussion_r182904205
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestImpl.java
---
@@ -0,0 +1,137 @@
+/*
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1223#discussion_r182902544
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/Unnest.java
---
@@ -0,0 +1,49 @@
+/*
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1223#discussion_r182908307
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestRecordBatch.java
---
@@ -0,0 +1,451
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1204
@oleg-zinovev good catch with the empty array input for Flatten. I had
reviewed the original JIRA DRILL-6099 but did not consider the empty arrays.
It would be good for @gparai to also take
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1191
+1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1173
@arina-ielchiieva could you rebase this on latest master ? thanks.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1157
+1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1141
+1.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1141#discussion_r171753536
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentStats.java ---
@@ -79,4 +71,21 @@ public void addOperatorStats(OperatorStats
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1141#discussion_r171723902
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentStats.java ---
@@ -79,4 +71,21 @@ public void addOperatorStats(OperatorStats
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r171711326
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
---
@@ -55,18 +62,21 @@ public void onMatch
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1096
Updated version lgtm. +1
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1141#discussion_r171611927
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentStats.java ---
@@ -79,4 +71,21 @@ public void addOperatorStats(OperatorStats
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r171479641
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java
---
@@ -224,4 +226,64 @@ public Void visitInputRef
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r171480227
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
---
@@ -55,18 +62,21 @@ public void onMatch
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r171478117
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java
---
@@ -224,4 +226,64 @@ public Void visitInputRef
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r171478085
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java
---
@@ -224,4 +226,64 @@ public Void visitInputRef
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1141#discussion_r171431227
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentStats.java ---
@@ -31,6 +32,13 @@
public class FragmentStats
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1081
@kkhatua since this was combined with #1082 which is merged, you can close
this PR.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1104
@arina-ielchiieva pls rebase on latest master.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/
+1. merged the PR in commit 27aa236.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1104
+1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1110
LGTM. +1
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1110#discussion_r168044397
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/SingleMergeExchangePrel.java
---
@@ -93,6 +94,21 @@ public
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1117
@ilooner thanks for making the changes. I have a minor comment on the test
query.
Otherwise LGTM. +1.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167722292
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoinAdvanced.java
---
@@ -197,4 +199,14 @@ public void
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1116
Updated version lgtm. +1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1110
@HanumathRao I have a few comments in the JIRA for the overall design; we
can discuss.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1110#discussion_r167447863
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOrderedMuxExchange.java
---
@@ -0,0 +1,116 @@
+/**
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1110#discussion_r167447232
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
---
@@ -20,133 +20,34
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1110#discussion_r167448218
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/OrderedMuxExchange.java
---
@@ -0,0 +1,64 @@
+/**
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1110#discussion_r167448543
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOrderedMuxExchange.java
---
@@ -0,0 +1,116 @@
+/**
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167441911
--- Diff:
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/TestHashJoinOrdering.java
---
@@ -0,0 +1,63 @@
+/**
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167442088
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/JoinPruleBase.java
---
@@ -246,4 +246,17 @@ public RelNode convertChild
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167442059
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/JoinPruleBase.java
---
@@ -246,4 +246,17 @@ public RelNode convertChild
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167441485
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentsRunner.java
---
@@ -61,7 +61,7 @@
private static final
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1117#discussion_r167441417
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoinAdvanced.java
---
@@ -197,4 +198,24 @@ public void
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1116#discussion_r167286191
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java
---
@@ -45,12 +45,25 @@
private final
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1116#discussion_r167288758
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java
---
@@ -45,12 +45,25 @@
private final
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1116#discussion_r167269862
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
---
@@ -322,4 +330,16 @@ public String
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1109
+1 . Thanks for adding the unit tests.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1109#discussion_r166981670
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinTemplate.java
---
@@ -117,9 +117,12 @@ private int
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r165788152
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
---
@@ -55,18 +62,21 @@ public void onMatch
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1096#discussion_r165791415
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
---
@@ -55,18 +62,21 @@ public void onMatch
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1106
Seems ok to fix the RecordBatchLoader.isSameSchema() since it is missing
the checks for nested columns. In order to do the consolidation that Paul
suggested, you might want to open
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1085
Sounds good. I will run the pre-commit tests. Since this PR was previously
in a ready-to-commit stage and only needed a rebase, it is fine to mark it
ready-to-commit.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1085
@paul-rogers since the DRILL-3993 (Calcite related changes) went into
master last week, this PR would need to be rebased. I can merge it in soon
after that.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1084
@kkhatua I saw some check style errors when applying the patch :
.git/rebase-apply/patch:215: trailing whitespace.
.git/rebase-apply/patch:396: trailing whitespace
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1059
@HanumathRao can you rebase your PR on the latest master and run the
functional tests ? I saw the following test failures which seem related to
these changes:
Execution Failures
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1066
Other than one comment above, rest LGTM. +1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1066
@vvysotskyi could you pls separate out the HashAggregate OOM related change
from this PR and file a separate JIRA for it since it is unrelated to the
calcite rebase per-se. Lumping them
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1066#discussion_r162171143
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/DrillRelBuilder.java
---
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1066#discussion_r157854723
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/DrillRelBuilder.java
---
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1066#discussion_r157853167
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/DrillRelBuilder.java
---
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1093
+1 . It would be good to add a unit test using the example in the JIRA.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1025#discussion_r150335985
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
---
@@ -795,6 +788,8 @@ private void
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1025#discussion_r150329246
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
---
@@ -795,6 +788,8 @@ private void
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1025
+1 with a minor comment. In the commit message and JIRA it would be
better to say 'code inspection' instead of code review which may be
interpreted to mean the normal code review process.
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1025#discussion_r150310556
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
---
@@ -177,11 +177,11 @@ public
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/889
@weijietong I am interested in getting your basic changes in. It is
unfortunate we are running into this issue with RelSubSet. Let me see if I can
make some changes on top of your changes (I
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/889
@weijietong I ran the functional tests with the PR and saw some failures.
While debugging those failures, I found the simple scalar aggregate case works
but anything with compound aggregate
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1007
Merged in 125a9271d7cf0dfb30aac8e62447507ea0a7d6c9. @HanumathRao pls close
the PR (for some reason I don't have permission).
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/996
Merged in 7a2fc87ee20f706d85cb5c90cc441e6b44b71592. @HanumathRao pls
close the PR.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1018
@arina-ielchiieva if this is good to go, can you mark the JIRA as
read-to-commit ? I will squash the 3 commits that @cgivre has created and
modify the commit message before committing.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/258
@cgivre regarding the commit process, you can send me a note at
amansi...@apache.org and I can walk through it. Although someone else
familiar with the process could do the commit, it would
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/258
Thanks @k255 . @cgivre once you have reviewed and are satisfied, pls mark
the JIRA as ready-to-commit label. Also, what do you both think about
documentation for these GIS functions ?
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/258
Somehow these extensions to the GIS module never got reviewed ! (the
original GIS functions are in the contrib). @k255 would you be able to rebase
your PR onto the latest master branch
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/979
+1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/932
Changes LGTM. +1
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/932#discussion_r141191139
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/spill/RecordBatchSizer.java
---
@@ -74,53 +74,52 @@
public final
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/959
lgtm. +1. @sohami could you pls check with @chunhui-shi if unit tests
were added previously for DRILL-4237 (another skew issue)? If not, could you
create a new JIRA to add such tests, which
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/905#discussion_r140417433
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdMaxRowCount.java
---
@@ -0,0 +1,71 @@
+/*
+* Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r140160556
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdMaxRowCount.java
---
@@ -0,0 +1,42 @@
+/*
+ * Licensed
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r140161176
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -203,32 +203,23 @@ public static void
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/926#discussion_r137619550
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/RecordReader.java ---
@@ -26,6 +26,9 @@
import
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r137295692
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -203,35 +203,27 @@ public static void
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r137291711
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -203,35 +203,27 @@ public static void
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/920
+1
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/909
Merged in d105950a7a9fb2ff3acd072ee65a51ef1fca120e. @vvysotskyi pls close
the PR (for some reason github is not showing me the option to close).
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/909
Thanks @vvysotskyi for the PR and @paul-rogers for reviewing the proposal
and code.
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/901
+1
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/931#discussion_r136856876
--- Diff:
contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBSubScan.java
---
@@ -71,14 +71,32 @@ public MapRDBSubScan
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/910
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/927
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/931#discussion_r136601989
--- Diff:
contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBSubScan.java
---
@@ -71,14 +71,32 @@ public MapRDBSubScan
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r135292416
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -231,6 +236,12 @@ public static boolean
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r135196236
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -204,24 +205,28 @@ public static void
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/889#discussion_r135199961
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
@@ -231,6 +236,12 @@ public static boolean
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/872
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/872#discussion_r128026173
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/AggPruleBase.java
---
@@ -61,11 +61,9 @@ protected AggPruleBase
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/840
Minor followup comment. LGTM. +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/840#discussion_r124958378
--- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
@@ -546,238 +576,400 @@ public DateTime getObject(int index
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/840#discussion_r124958090
--- Diff: exec/vector/src/main/codegen/templates/NullableValueVectors.java
---
@@ -674,6 +764,14 @@ public void reset(){
setCount = 0
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/840#discussion_r124710371
--- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
@@ -546,238 +576,400 @@ public DateTime getObject(int index
1 - 100 of 487 matches
Mail list logo
