[GitHub] spark issue #22827: [SPARK-25832][SQL][BRANCH-2.4] Revert newly added map re...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22827 thanks, merging to 2.4! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark pull request #22029: [SPARK-24395][SQL] IN operator should return NULL...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22029#discussion_r228245697 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala --- @@ -202,7 +209,11 @@ case class InSubquery(values

[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22814 LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #22812: [SPARK-25817][SQL] Dataset encoder should support combin...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22812 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark pull request #22809: [SPARK-19851][SQL] Add support for EVERY and ANY ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22809#discussion_r228238508 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Max.scala --- @@ -57,3 +57,34 @@ case class Max(child

[GitHub] spark pull request #22809: [SPARK-19851][SQL] Add support for EVERY and ANY ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22809#discussion_r228238276 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Max.scala --- @@ -57,3 +57,34 @@ case class Max(child

[GitHub] spark issue #22675: [SPARK-25347][ML][DOC] Spark datasource for image/libsvm...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22675 thanks, merging to master/2.4! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark issue #22790: [SPARK-25793][ML]call SaveLoadV2_0.load for classNameV2_...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22790 Is this ready to go? We are going to have another RC, and would be good to include it. --- - To unsubscribe, e-mail: reviews

[GitHub] spark issue #22775: [SPARK-24709][SQL][FOLLOW-UP] Make schema_of_json's inpu...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22775 I think I'm not qualified to make the decision here, as I don't fully understand the use case. It looks to me that one use case would be to run `schema_of_json` on a column and manually

[GitHub] spark issue #22825: [SPARK-25772][SQL][FOLLOWUP] remove GetArrayFromMap

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22825 cc @vofque @viirya --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark pull request #22825: [SPARK-25772][SQL][FOLLOWUP] remove GetArrayFromM...

2018-10-25 Thread cloud-fan
GitHub user cloud-fan opened a pull request: https://github.com/apache/spark/pull/22825 [SPARK-25772][SQL][FOLLOWUP] remove GetArrayFromMap ## What changes were proposed in this pull request? In https://github.com/apache/spark/pull/22745 we introduced the `GetArrayFromMap

[GitHub] spark pull request #22812: [SPARK-25817][SQL] Dataset encoder should support...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22812#discussion_r228195016 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoder.scala --- @@ -119,10 +119,9 @@ object ExpressionEncoder

[GitHub] spark issue #22821: [SPARK-25832][SQL] remove newly added map related functi...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22821 @dongjoon-hyun if there are advanced users who know all the background, and still want to use these functions, why shall we stop them? If end users can't hit the bug with public APIs, I think we

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228164819 --- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroCatalystDataConversionSuite.scala --- @@ -167,4 +184,26 @@ class

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228164087 --- Diff: docs/sql-migration-guide-upgrade.md --- @@ -11,6 +11,10 @@ displayTitle: Spark SQL Upgrading Guide - In PySpark, when creating

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228162464 --- Diff: docs/sql-migration-guide-upgrade.md --- @@ -11,6 +11,10 @@ displayTitle: Spark SQL Upgrading Guide - In PySpark, when creating

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228160490 --- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroFunctionsSuite.scala --- @@ -61,6 +59,24 @@ class AvroFunctionsSuite extends

[GitHub] spark issue #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder to get ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22749 thanks, merging to master! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r228134985 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoder.scala --- @@ -212,21 +181,91 @@ object ExpressionEncoder

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r228132840 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoder.scala --- @@ -212,21 +181,91 @@ object ExpressionEncoder

[GitHub] spark issue #22621: [SPARK-25602][SQL] SparkPlan.getByteArrayRdd should not ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22621 why do we need migration guide for bug fix? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228063724 --- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroCatalystDataConversionSuite.scala --- @@ -17,30 +17,49 @@ package

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228063256 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroOptions.scala --- @@ -79,4 +80,23 @@ class AvroOptions( val compression

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228062884 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/package.scala --- @@ -31,10 +32,32 @@ package object avro { * @since 2.4.0

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228062545 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -44,24 +59,74 @@ case class AvroDataToCatalyst(child

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228061155 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -44,24 +59,74 @@ case class AvroDataToCatalyst(child

[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...

2018-10-25 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22814#discussion_r228059940 --- Diff: docs/sql-data-sources-avro.md --- @@ -177,6 +180,18 @@ Data source options of Avro can be set using the `.option` method on `DataFrameR

[GitHub] spark issue #22821: [SPARK-25832][SQL] remove newly added map related functi...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22821 cc @dongjoon-hyun @gatorsmile @rxin --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark pull request #22821: [SPARK-25832][] remove newly added map related fu...

2018-10-24 Thread cloud-fan
GitHub user cloud-fan opened a pull request: https://github.com/apache/spark/pull/22821 [SPARK-25832][] remove newly added map related functions from FunctionRegistry ## What changes were proposed in this pull request? (Please fill in changes proposed in this fix

[GitHub] spark issue #22773: [SPARK-25785][SQL] Add prettyNames for from_json, to_jso...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22773 I think the new names are better and expected, though it's safer to mention it in the migration guide in case some users care about

[GitHub] spark issue #22755: [SPARK-25755][SQL][Test] Supplementation of non-CodeGen ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22755 +1 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #22514: [SPARK-25271][SQL] Hive ctas commands should use ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22514#discussion_r228013784 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/CreateHiveTableAsSelectCommand.scala --- @@ -45,6 +46,11 @@ case class

[GitHub] spark issue #22809: [SPARK-19851][SQL] Add support for EVERY and ANY (SOME) ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22809 LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #22809: [SPARK-19851][SQL] Add support for EVERY and ANY ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22809#discussion_r228013503 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala --- @@ -727,4 +728,67 @@ class DataFrameAggregateSuite extends

[GitHub] spark issue #22809: [SPARK-19851][SQL] Add support for EVERY and ANY (SOME) ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22809 Can we use these functions in window with this approach? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark pull request #22809: [SPARK-19851][SQL] Add support for EVERY and ANY ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22809#discussion_r227899342 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala --- @@ -38,6 +39,18 @@ object ReplaceExpressions

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 Maybe we should discuss it in another thread, I think we should officially write down the procedure about how to evaluate a bug report and label it as a blocker or not. Currently https

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227867735 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/ScalaReflectionSuite.scala --- @@ -280,59 +281,59 @@ class ScalaReflectionSuite

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 @tgravescs please quote my full comment instead of part of it. > After all, this is a bug and a regression from previous releases, like other 1000 we've fixed before. The poin

[GitHub] spark issue #22730: [SPARK-16775][CORE] Remove deprecated accumulator v1 API...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22730 A late LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227783724 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/ScalaReflectionSuite.scala --- @@ -280,59 +281,59 @@ class ScalaReflectionSuite

[GitHub] spark issue #22237: [SPARK-25243][SQL] Use FailureSafeParser in from_json

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22237 thanks, merging to master! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark issue #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder to get ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22749 LGTM except 2 minor comments --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227742062 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/ScalaReflectionSuite.scala --- @@ -351,11 +347,15 @@ class ScalaReflectionSuite

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227739775 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoder.scala --- @@ -212,21 +181,90 @@ object ExpressionEncoder

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227682823 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala --- @@ -58,12 +58,10 @@ object RowEncoder { def

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227682672 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala --- @@ -434,17 +426,34 @@ object ScalaReflection extends

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227681844 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala --- @@ -1087,7 +1087,7 @@ class Dataset[T] private[sql]( // Note that we do

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227675880 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala --- @@ -58,12 +58,10 @@ object RowEncoder { def

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227673675 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala --- @@ -434,17 +426,34 @@ object ScalaReflection extends

[GitHub] spark pull request #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22749#discussion_r227672066 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala --- @@ -434,17 +426,34 @@ object ScalaReflection extends

[GitHub] spark issue #22237: [SPARK-25243][SQL] Use FailureSafeParser in from_json

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22237 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark issue #22812: [SPARK-25817][SQL] Dataset encoder should support combin...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22812 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark issue #22749: [SPARK-25746][SQL] Refactoring ExpressionEncoder to get ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22749 hmm, it still has conflict... --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark pull request #21860: [SPARK-24901][SQL]Merge the codegen of RegularHas...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/21860#discussion_r227655432 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala --- @@ -831,7 +832,14 @@ case class HashAggregateExec

[GitHub] spark pull request #22514: [SPARK-25271][SQL] Hive ctas commands should use ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22514#discussion_r227654755 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/CreateHiveTableAsSelectCommand.scala --- @@ -45,6 +46,11 @@ case class

[GitHub] spark pull request #22514: [SPARK-25271][SQL] Hive ctas commands should use ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22514#discussion_r227654240 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/CreateHiveTableAsSelectCommand.scala --- @@ -45,6 +46,11 @@ case class

[GitHub] spark pull request #22514: [SPARK-25271][SQL] Hive ctas commands should use ...

2018-10-24 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22514#discussion_r227653464 --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSuite.scala --- @@ -92,4 +92,18 @@ class HiveParquetSuite extends QueryTest

[GitHub] spark pull request #21860: [SPARK-24901][SQL]Merge the codegen of RegularHas...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/21860#discussion_r227650326 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala --- @@ -831,7 +832,14 @@ case class HashAggregateExec

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 Unfortunately, we didn't drop it mistakenly. It's a mistake and we should fix it. What I try to avoid is adding back the `supportsPartial` flag. We should look into the root cause and see how

[GitHub] spark issue #22237: [SPARK-25243][SQL] Use FailureSafeParser in from_json

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22237 LGTM, pending jenkins. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark issue #22812: [SPARK-25817][SQL] Dataset encoder should support combin...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22812 cc @michalsenkyr @vofque @viirya --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands

[GitHub] spark pull request #22812: [SPARK-25817][SQL] Dataset encoder should support...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22812#discussion_r227638941 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1837,8 +1837,6 @@ case class

[GitHub] spark pull request #22812: [SPARK-25817][SQL] Dataset encoder should support...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22812#discussion_r227638902 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1090,15 +1096,9 @@ case class

[GitHub] spark pull request #22812: [SPARK-25817][SQL] Dataset encoder should support...

2018-10-23 Thread cloud-fan
GitHub user cloud-fan opened a pull request: https://github.com/apache/spark/pull/22812 [SPARK-25817][SQL] Dataset encoder should support combination of map and product type ## What changes were proposed in this pull request? After https://github.com/apache/spark/pull

[GitHub] spark issue #22514: [SPARK-25271][SQL] Hive ctas commands should use data so...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22514 sounds like a clean solution. please go ahead, thanks! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark pull request #22512: [SPARK-25498][SQL] InterpretedMutableProjection s...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22512#discussion_r227618778 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MutableProjectionSuite.scala --- @@ -0,0 +1,66

[GitHub] spark pull request #22512: [SPARK-25498][SQL] InterpretedMutableProjection s...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22512#discussion_r227618313 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/InternalRow.scala --- @@ -143,4 +144,25 @@ object InternalRow { case u

[GitHub] spark issue #22745: [SPARK-25772][SQL] Fix java map of structs deserializati...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22745 thanks, merging to master! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227611044 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1788,78 @@ case class

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 After all, this is a bug and a regression from previous releases, like other 1000 we've fixed before. According to the policy, we don't have to block the current release because

[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22788 I agree with the problem described in the PR description that `UnresolvedAttribute.sql` is not ideal. But we should just update `UnresolvedAttribute.sql`, not the `name` method. `name` is used

[GitHub] spark issue #22514: [SPARK-25271][SQL] Hive ctas commands should use data so...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22514 @viirya can you explain the high-level idea about how to fix it? It seems hard to fix and we should get a consensus on the approach first

[GitHub] spark pull request #22514: [SPARK-25271][SQL] Hive ctas commands should use ...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22514#discussion_r227602783 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala --- @@ -34,11 +34,16 @@ import org.apache.spark.sql.types

[GitHub] spark issue #22514: [SPARK-25271][SQL] Hive ctas commands should use data so...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22514 Yes this is a performance regression for users who run CTAS on Hive serde tables. This is a regression since Spark 2.3.0

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 This is not a PR that is ready to merge. We are likely talking about delaying 2.4.0 for multiple weeks because of this issue. Is it really worth? I'm not sure what's the exact policy

[GitHub] spark pull request #22512: [SPARK-25498][SQL] InterpretedMutableProjection s...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22512#discussion_r227433044 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/InternalRow.scala --- @@ -143,4 +144,24 @@ object InternalRow { case u

[GitHub] spark pull request #22512: [SPARK-25498][SQL] InterpretedMutableProjection s...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22512#discussion_r227432603 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/InternalRow.scala --- @@ -143,4 +144,24 @@ object InternalRow { case u

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227431215 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1788,79 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227430882 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1788,79 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227407344 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,72 @@ case class

[GitHub] spark issue #22785: [SPARK-25791][SQL] Datatype of serializers in RowEncoder...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22785 thanks, merging to master! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 Note that the `supportsPartial` flag was dropped at Spark 2.2, not 2.4. I'm not very familiar with Hive code so I don't clearly know how it is broken. The worst case is, Hive has some

[GitHub] spark issue #22745: [SPARK-25772][SQL] Fix java map of structs deserializati...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22745 LGTM except 2 minor comments --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227391572 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,72 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227391434 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,72 @@ case class

[GitHub] spark issue #22237: [SPARK-25243][SQL] Use FailureSafeParser in from_json

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22237 @HyukjinKwon are you working on it? @gengliangwang do you want to take over? --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227357201 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,75 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227356448 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,75 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227355778 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,75 @@ case class

[GitHub] spark pull request #22745: [SPARK-25772][SQL] Fix java map of structs deseri...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22745#discussion_r227355335 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -1787,3 +1787,75 @@ case class

[GitHub] spark issue #22144: [SPARK-24935][SQL] : Problem with Executing Hive UDF's f...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22144 My feeling is that, hive compatibility is not that important to Spark at this point. *ALL* aggregate functions in Spark (including Spark UDAF) support partial aggregate, but now we need

[GitHub] spark pull request #22778: [SPARK-25784][SQL] Infer filters from constraints...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22778#discussion_r227261253 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala --- @@ -171,9 +171,11 @@ abstract class Optimizer

[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22788 yea, only use json if it's a nested column. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark pull request #21156: [SPARK-24087][SQL] Avoid shuffle when join keys a...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/21156#discussion_r227253732 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/joins/JoinUtils.scala --- @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache

[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22788 Yea I think so, we can even use JSON to be safer. e.g. for `a.b.c.d`, we can encode it as a json array [a,b,c,d]. At data source side, use a json parser to read it back

[GitHub] spark issue #22797: [SPARK-19851][SQL] Add support for EVERY and ANY (SOME) ...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22797 yea, just a special rule instead of a general agg function rewrite framework. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark pull request #22775: [SPARK-24709][SQL][FOLLOW-UP] Make schema_of_json...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22775#discussion_r227243187 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala --- @@ -770,8 +776,17 @@ case class SchemaOfJson

[GitHub] spark issue #22799: [SPARK-25805][SQL][TEST] Fix test for SPARK-25159

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22799 thanks, merging to master/2.4! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark pull request #22512: [SPARK-25498][SQL] InterpretedMutableProjection s...

2018-10-23 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22512#discussion_r22723 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/InterpretedMutableProjection.scala --- @@ -49,10 +51,54 @@ class

<    3   4   5   6   7   8   9   10   11   12   >