Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21845
@HyukjinKwon do we have any idea why we are hitting a timeout?
---
-
To unsubscribe, e-mail: reviews-unsubscr
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16677
@mridulm are you ok with the current changes?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21826
I am not sure there is much to do here. `OR` is the standard syntax for or,
and `||` is pretty common syntax for string concatenation. Removing `||` breaks
binary compatibility and also makes
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21822#discussion_r204190535
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -231,10 +231,11 @@ class Analyzer
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21822#discussion_r204167870
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
---
@@ -533,7 +537,8 @@ trait CheckAnalysis extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21822#discussion_r204163551
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
---
@@ -533,7 +537,8 @@ trait CheckAnalysis extends
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21806
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21807
@NiharS yeah that makes sense. @mauropalsgraaf we missed this today (sorry
about that). Can you add the null check (bonus points if you call `eval()` only
once), add a test for this case
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21806
@viirya this current change is only useful when you compare canonicalized
plans created on different JVMs. This has come up when we tried to detect
changes in plans over spark versions (plan
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21806
otherwise, LGTM pending jenkins
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21806
@gvr can you clean-up the description somewhat? It currently also has part
of the template in it.
---
-
To unsubscribe, e
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21807
Ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21806
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21772
@liutang123 can you explain why we are losing data when serializing to
disk. Also can you add a unit test
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21772
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21728
LGTM
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21738#discussion_r201154124
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -563,4 +563,12 @@ package object config {
.intConf
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21727
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21680
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r198106419
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala
---
@@ -193,6 +193,16 @@ case object
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21634
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21634#discussion_r197841906
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala
---
@@ -504,4 +504,38 @@ class SQLMetricsSuite extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21634#discussion_r197803612
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala
---
@@ -504,4 +504,38 @@ class SQLMetricsSuite extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21634#discussion_r197803317
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala
---
@@ -504,4 +504,38 @@ class SQLMetricsSuite extends
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21634
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21634
add to whitelist
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r197410930
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/limit.scala ---
@@ -93,25 +98,95 @@ trait BaseLimitExec extends UnaryExecNode
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r197410511
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala
---
@@ -193,6 +193,16 @@ case object
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r197284779
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/limit.scala ---
@@ -93,25 +98,101 @@ trait BaseLimitExec extends UnaryExecNode
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r197117604
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala
---
@@ -193,6 +193,16 @@ case object
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16677#discussion_r197116936
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala
---
@@ -247,6 +253,10 @@ object ShuffleExchangeExec
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16677
@viirya yes we should. Sorry for not getting back to this quicker.
---
-
To unsubscribe, e-mail: reviews-unsubscr
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21580
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21595
LGTM - merging to master. Thanks
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21580
LGTM. It is better UX to have a more descriptive error messages.
However I do like the idea of being able to use window functions in
filters. I often use the following pattern
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21573
LGTM - Merging to 2.3. Thanks!
What is the process for master?
---
-
To unsubscribe, e-mail: reviews-unsubscr
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21388
@HeartSaVioR I really don't think we should automate these things at all.
The planner is a pretty critical component, and I'd rather be explicit on how a
`LogicalPlan` maps to a `SparkPlan
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21539#discussion_r194902636
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala ---
@@ -679,6 +679,13 @@ class PlannerSuite extends SharedSQLContext
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21539#discussion_r194902354
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -301,6 +290,37 @@ abstract class TreeNode[BaseType
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21535
@viirya how much time this add to the overall testing time?
If this is too much then we should perhaps just test interpreted encoders.
WDYT
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21539
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21539
cc @bogdanrdc @cloud-fan @ssimeonov
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
GitHub user hvanhovell opened a pull request:
https://github.com/apache/spark/pull/21539
[SPARK-24500][SQL] Make sure streams are materialized during Tree
transforms.
## What changes were proposed in this pull request?
If you construct catalyst trees using
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21508#discussion_r194023397
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -1568,11 +1568,13 @@ class Analyzer
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21508
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21402
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21484#discussion_r192564074
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -390,7 +390,7 @@ class Dataset[T] private[sql
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21082#discussion_r191740985
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/python/WindowInPandasExec.scala
---
@@ -0,0 +1,173 @@
+/*
+ * Licensed
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21425#discussion_r190841865
--- Diff:
sql/core/src/test/resources/sql-tests/inputs/subquery/in-subquery/not-in-unit-tests-single-column.sql
---
@@ -0,0 +1,123 @@
+-- Unit
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21425#discussion_r190841604
--- Diff:
sql/core/src/test/resources/sql-tests/results/typeCoercion/native/concat.sql.out
---
@@ -1,5 +1,5 @@
-- Automatically generated
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21403
@mgaido91 can you link the correct JIRA? This one does not seem to be the
correct one.
---
-
To unsubscribe, e-mail
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21403#discussion_r190160896
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
---
@@ -2261,6 +2261,24 @@ class DataFrameSuite extends QueryTest
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21403#discussion_r190160677
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala
---
@@ -45,6 +46,10 @@ object RewritePredicateSubquery
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21388
I donât think it is a good a idea to put reflection magic in the planner.
If you want to add cases to the planner please use the existing hooks
(SparkSessionExtensions, ExperimentalMethods
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21371
@viirya the test failure seems to genuine, can you take another look?
---
-
To unsubscribe, e-mail: reviews-unsubscr
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19193
@cloud-fan and @aokolnychyi I am ok with supporting this, especially since
most of the work is already done. WDYT
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r189433380
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/window/WindowExec.scala
---
@@ -213,10 +218,24 @@ case class WindowExec
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21330#discussion_r189433323
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -1883,7 +1883,19 @@ class Analyzer
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21299#discussion_r187618769
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/ReadOnlySQLConf.scala
---
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21276
ok to test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186673964
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
---
@@ -147,7 +148,44 @@ case class SortPrefix(child
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186670840
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
---
@@ -114,6 +114,113 @@ object JavaCode
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186670675
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
---
@@ -114,6 +114,113 @@ object JavaCode
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186669860
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -119,7 +121,7 @@ abstract class Expression
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21149
LGTM - merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186668643
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
---
@@ -147,7 +148,44 @@ case class SortPrefix(child
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186644321
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
---
@@ -623,8 +624,14 @@ case class Cast(child: Expression
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21144
LGTM - merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14083
Merging to master. Thanks for all the reviews!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14083
Done.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186361480
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
---
@@ -112,6 +112,112 @@ object JavaCode
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21193#discussion_r186356233
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/ExprValueSuite.scala
---
@@ -17,6 +17,8 @@
package
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21225
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21082
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21082
It should be fixed. Let's retry.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21227
LGTM - merging to 2.3. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21225#discussion_r185821003
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/window/WindowExec.scala
---
@@ -197,6 +198,7 @@ case class WindowExec
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21210
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21064
@kiszk sorry about the delay.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21206
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21193
@viirya I just glanced over this. We currently use a lot of mutable code
for in code generation with - TBH - way too complex side effects. Your proposal
adds another layer of side effects
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21139#discussion_r183893505
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -618,6 +619,18 @@ object CollapseRepartition
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21144
@bersprockets can you also check if other interpreted code paths have this
problem?
---
-
To unsubscribe, e-mail: reviews
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21144#discussion_r183890298
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -36,6 +36,14 @@ object InterpretedPredicate
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21137
Merging to master.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21137#discussion_r183652913
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -1267,12 +1274,12 @@ case class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21137#discussion_r183652630
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -1255,6 +1255,13 @@ case class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21106
Yeah, let's move forward slowly. I am still pondering about the what the
right abstraction here would look like; this looks promising though.
Can you try to unify this class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20980#discussion_r183374589
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -1255,8 +1255,64 @@ case class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20980#discussion_r183374541
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -1255,8 +1255,64 @@ case class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20980#discussion_r183373874
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
---
@@ -501,6 +502,111 @@ class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14083
@cloud-fan can you take a look?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20757
LGTM - merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21112
Merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20980
@maropu one more thing, otherwise LGTM.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20980#discussion_r183036460
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
---
@@ -472,6 +474,61 @@ class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21112
LGTM
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r182820983
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala
---
@@ -80,30 +80,44 @@ case class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20636
@kiszk Why do we need to allocate a large array several times? I thought
the objective of this test is to check if we can safely grow to int max? I
don't really see the need for resetting
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/20046
LGTM - merging to master. Thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands
101 - 200 of 4165 matches
Mail list logo
