[GitHub] spark issue #22683: [SPARK-25696] The storage memory displayed on spark Appl...

2018-12-10 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22683 In this case the change is simpler to understand in prose, I think; "100 KB" becomes "97.6 KiB", etc. --- ---

[GitHub] spark pull request #23275: [SPARK-26323][SQL] Scala UDF should still check i...

2018-12-10 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23275#discussion_r240234573 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ScalaUDF.scala --- @@ -47,25 +47,13 @@ case class ScalaUDF

[GitHub] spark pull request #23275: [SPARK-26323][SQL] Scala UDF should still check i...

2018-12-10 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23275#discussion_r240234371 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ScalaUDF.scala --- @@ -47,25 +47,13 @@ case class ScalaUDF

[GitHub] spark issue #23260: [SPARK-26311][YARN] New feature: custom log URL for stdo...

2018-12-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23260 Ok, got it. @vanzin or @squito or others would be better able to evaluate. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark issue #23072: [SPARK-19827][R]spark.ml R API for PIC

2018-12-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23072 @dongjoon-hyun @felixcheung how about now? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #23260: [SPARK-26311][YARN] New feature: custom log URL for stdo...

2018-12-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23260 If you're on YARN, this feels like something you would manage via YARN and its cluster management options. Is there a specific use case here, that this has to happen in Spark

[GitHub] spark issue #23263: [SPARK-23674][ML] Adds Spark ML Events

2018-12-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23263 My first impression is that it's a big change, which is reason for caution here. Visualizing a workflow is nice, but Spark's Pipelines are typically pretty straightforward and linear. I

[GitHub] spark issue #23241: [SPARK-26283][CORE] Enable reading from open frames of z...

2018-12-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23241 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #22683: [SPARK-25696] The storage memory displayed on spark Appl...

2018-12-08 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22683 Fortunately the syntax is "100m", which has always meant "100 * 1024 * 1024" or "100 MiB" --- - T

[GitHub] spark pull request #23264: [SPARK-26266][BUILD] Update to Scala 2.12.8 (bran...

2018-12-08 Thread srowen
Github user srowen closed the pull request at: https://github.com/apache/spark/pull/23264 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #23264: Update to Scala 2.12.8

2018-12-08 Thread srowen
GitHub user srowen opened a pull request: https://github.com/apache/spark/pull/23264 Update to Scala 2.12.8 ## What changes were proposed in this pull request? Back-port of https://github.com/apache/spark/pull/23218 ; updates Scala 2.12 build to 2.12.8 ## How

[GitHub] spark issue #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-08 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23218 Merged to master. I'll open a separate PR for branch-2.4 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark pull request #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-08 Thread srowen
Github user srowen closed the pull request at: https://github.com/apache/spark/pull/23218 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #23225: [SPARK-26287][CORE]Don't need to create an empty ...

2018-12-07 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23225#discussion_r239990036 --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/ShuffleExternalSorter.java --- @@ -161,6 +161,10 @@ private void writeSortedFile(boolean

[GitHub] spark issue #23256: [SPARK-24207][R] follow-up PR for SPARK-24207 to fix cod...

2018-12-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23256 CC @felixcheung --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #23160: [SPARK-26196][SPARK-26281][WebUI] Total tasks title in t...

2018-12-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23160 Merged to master. @shahidki31 does this need to go in branch 2.4, 2.3? I tried back porting it, but looks like a lot of the affected code didn't exist in 2.4. If the fix can or should also be back

[GitHub] spark issue #23247: [SPARK-26294][CORE]Delete Unnecessary If statement

2018-12-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23247 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #17673: [SPARK-20372] [ML] Word2Vec Continuous Bag of Words mode...

2018-12-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/17673 @ngopal this one can't be merged as-is and looks like it was abandoned. Would you like to take this PR, update per reviews? I'd review that. I think CBOW could be useful in MLlib

[GitHub] spark issue #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23218 I'm not sure. I can't find any other reference to this crash and 2.12.8. It could be something only Spark happens to trigger, or could be specific to this JVM + platform but not Spark or Scala. We

[GitHub] spark issue #23241: [SPARK-26283][CORE] Enable reading from open frames of z...

2018-12-06 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23241 Sorry about the run-around. I'm OK being conservative here as you were originally, too. --- - To unsubscribe, e-mail: reviews

[GitHub] spark issue #23246: [SPARK-26292][CORE]Assert statement of currentPage may b...

2018-12-06 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23246 It's not clear this is where it should be from the description. Please review https://spark.apache.org/contributing.html This one should be closed

[GitHub] spark issue #23247: [SPARK-26294][CORE]Delete Unnecessary If statement

2018-12-06 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23247 These aren't worth the time it takes us to review them and merge them, honestly. Little cleanup can be OK if it makes an appreciable difference in speed or readability, and if you can find many

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239590339 --- Diff: core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala --- @@ -118,8 +118,6 @@ private[spark] class ReplayListenerBus extends

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239532748 --- Diff: core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala --- @@ -118,10 +118,12 @@ private[spark] class ReplayListenerBus extends

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239525888 --- Diff: core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala --- @@ -118,10 +118,12 @@ private[spark] class ReplayListenerBus extends

[GitHub] spark issue #23202: [SPARK-26248][SQL] Infer date type from CSV

2018-12-06 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23202 I'd defer to @HyukjinKwon ; looks OK in broad strokes but he would know much more about the CSV parsing. --- - To unsubscribe, e

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239509724 --- Diff: core/src/main/scala/org/apache/spark/io/CompressionCodec.scala --- @@ -197,4 +201,8 @@ class ZStdCompressionCodec(conf: SparkConf) extends

[GitHub] spark pull request #22683: [SPARK-25696] The storage memory displayed on spa...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22683#discussion_r239482033 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/NullExpressionsSuite.scala --- @@ -160,7 +160,7 @@ class

[GitHub] spark pull request #22683: [SPARK-25696] The storage memory displayed on spa...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22683#discussion_r239480795 --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala --- @@ -62,14 +62,14 @@ class KryoSerializer(conf: SparkConf

[GitHub] spark pull request #22683: [SPARK-25696] The storage memory displayed on spa...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22683#discussion_r239481508 --- Diff: docs/sql-programming-guide.md --- @@ -4,10 +4,15 @@ displayTitle: Spark SQL, DataFrames and Datasets Guide title: Spark SQL and DataFrames

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239476570 --- Diff: core/src/main/scala/org/apache/spark/io/CompressionCodec.scala --- @@ -197,4 +201,8 @@ class ZStdCompressionCodec(conf: SparkConf) extends

[GitHub] spark pull request #23241: [SPARK-26283][CORE] Enable reading from open fram...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239476672 --- Diff: core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala --- @@ -118,8 +118,6 @@ private[spark] class ReplayListenerBus extends

[GitHub] spark pull request #22759: [MINOR][SQL][DOC] Correct parquet nullability doc...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22759#discussion_r239474962 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/test/DataFrameReaderWriterSuite.scala --- @@ -542,6 +551,35 @@ class DataFrameReaderWriterSuite

[GitHub] spark pull request #22759: [MINOR][SQL][DOC] Correct parquet nullability doc...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22759#discussion_r239475332 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/test/DataFrameReaderWriterSuite.scala --- @@ -542,6 +551,35 @@ class DataFrameReaderWriterSuite

[GitHub] spark pull request #22759: [MINOR][SQL][DOC] Correct parquet nullability doc...

2018-12-06 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22759#discussion_r239475203 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/test/DataFrameReaderWriterSuite.scala --- @@ -542,6 +551,35 @@ class DataFrameReaderWriterSuite

[GitHub] spark pull request #23241: [SPARK-26283][CORE]When zstd compression enabled,...

2018-12-05 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239218209 --- Diff: core/src/main/scala/org/apache/spark/io/CompressionCodec.scala --- @@ -197,4 +201,8 @@ class ZStdCompressionCodec(conf: SparkConf) extends

[GitHub] spark pull request #23228: [MINOR][DOC]The condition description of serializ...

2018-12-05 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23228#discussion_r239215561 --- Diff: core/src/main/scala/org/apache/spark/shuffle/sort/SortShuffleManager.scala --- @@ -33,10 +33,10 @@ import org.apache.spark.shuffle._ * Sort

[GitHub] spark pull request #23241: [SPARK-26283][CORE]When zstd compression enabled,...

2018-12-05 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23241#discussion_r239208072 --- Diff: core/src/main/scala/org/apache/spark/io/CompressionCodec.scala --- @@ -197,4 +201,8 @@ class ZStdCompressionCodec(conf: SparkConf) extends

[GitHub] spark issue #23231: [SPARK-26273][ML] Add OneHotEncoderEstimator as alias to...

2018-12-05 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23231 I'm not seeing it in the migration guide, maybe I'm missing it. In any event, I dont' think we need to keep this for 3.0

[GitHub] spark issue #23229: [MINOR][CORE] Modify some field name because it may be c...

2018-12-05 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23229 Agree, this isn't worthwhile. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail

[GitHub] spark pull request #23196: [SPARK-26243][SQL] Use java.time API for parsing ...

2018-12-05 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23196#discussion_r239068840 --- Diff: sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala --- @@ -49,8 +49,8 @@ class

[GitHub] spark issue #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-05 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23218 Ah OK, so all of them were a JVM crash. It would probably be a good idea to update the JVM on all the workers as _60 is over 3 years old. It's probably not as simple as it sounds but WDYT

[GitHub] spark issue #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23218 Hm, one failure was due to a JVM crash, but it fails twice consistent, with sbt just exiting with status 134. No other failures are logged. Not sure what to make

[GitHub] spark issue #23216: [SPARK-26264][CORE]It is better to add @transient to fie...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23216 I think just leave it. The `@transient` in `ShuffleMapTasks`'s `locs` is just superfluous here, not sure it's worth changing

[GitHub] spark issue #23216: [SPARK-26264][CORE]It is better to add @transient to fie...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23216 Are you sure it's even a field in the class? it looks like it's only used to define this: ``` @transient private[this] val preferredLocs: Seq[TaskLocation] = { if (locs == null

[GitHub] spark pull request #23159: [SPARK-26191][SQL] Control truncation of Spark pl...

2018-12-04 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23159#discussion_r238869530 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala --- @@ -1777,7 +1777,7 @@ class Analyzer( case

[GitHub] spark issue #23159: [SPARK-26191][SQL] Control truncation of Spark plans via...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23159 Rather than change every single call to this method, if this should generally be the value of the argument, then why not make it the default value or something

[GitHub] spark issue #23219: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23219 @wangyum I already opened https://github.com/apache/spark/pull/23218 --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark issue #22759: [MINOR][SQL][DOC] Correct parquet nullability documentat...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22759 Ping @dima-asana to rebase or close --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e

[GitHub] spark issue #21363: [SPARK-19228][SQL] Migrate on Java 8 time from FastDateF...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/21363 @MaxGekk now that your change is merge, can this proceed, @xuanyuanking ? or is it obsolete? --- - To unsubscribe, e-mail

[GitHub] spark issue #22997: SPARK-25999: make-distribution.sh failure with --r and -...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22997 Yeah, we can't make this change for the reasons above. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark issue #22887: [SPARK-25880][CORE] user set's hadoop conf should not ov...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22887 @gjhkael can you clarify further what the undesirable behavior is, and what behavior you are looking for? --- - To unsubscribe

[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23098 Note I'm holding on to this PR for a while as I understand it might be disruptive to downstream builds to remove 2.11 support just now. Will look at merging it in weeks. Right now it's an FYI

[GitHub] spark issue #23150: [SPARK-26178][SQL] Use java.time API for parsing timesta...

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23150 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23218 Hm, looks like genjavadocplugin is published for individual Scala releases and doesn't exist yet for 2.12.8: https://mvnrepository.com/artifact/com.typesafe.genjavadoc/genjavadoc-plugin . I'll look

[GitHub] spark issue #23170: [SPARK-24423][FOLLOW-UP][SQL] Fix error example

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23170 Merged to master/2.4 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark issue #22600: [SPARK-25578][BUILD] Update to Scala 2.12.7

2018-12-04 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22600 @wangyum sounds good. I opened https://github.com/apache/spark/pull/23218 --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark pull request #23218: [SPARK-26266][BUILD] Update to Scala 2.12.8

2018-12-04 Thread srowen
GitHub user srowen opened a pull request: https://github.com/apache/spark/pull/23218 [SPARK-26266][BUILD] Update to Scala 2.12.8 ## What changes were proposed in this pull request? Update to Scala 2.12.8 ## How was this patch tested? Existing tests. You

[GitHub] spark issue #23182: Config change followup to [SPARK-26177] Automated format...

2018-12-03 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23182 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #23189: [SPARK-26235][Core] Change log level for ClassNotFoundEx...

2018-12-03 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23189 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #18784: [SPARK-21559][Mesos] remove mesos fine-grained mode

2018-12-03 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/18784 @skonto do you want to proceed with this? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #22683: [SPARK-25696] The storage memory displayed on spark Appl...

2018-12-03 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22683 Add to this PR. The change goes logically together. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark issue #23205: [SPARK-26253][WebUI] : Task Summary Metrics Table on Sta...

2018-12-03 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23205 Merging as a follow up to https://github.com/apache/spark/pull/21688 --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark issue #23164: [SPARK-26198][SQL] Fix Metadata serialize null values th...

2018-12-02 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23164 OK merged to 2.4/2.3 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark pull request #23088: [SPARK-26119][CORE][WEBUI]Task summary table shou...

2018-12-02 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23088#discussion_r238110710 --- Diff: core/src/main/scala/org/apache/spark/status/AppStatusStore.scala --- @@ -148,11 +148,20 @@ private[spark] class AppStatusStore

[GitHub] spark pull request #23088: [SPARK-26119][CORE][WEBUI]Task summary table shou...

2018-12-02 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23088#discussion_r238110723 --- Diff: core/src/main/scala/org/apache/spark/status/AppStatusStore.scala --- @@ -221,29 +230,49 @@ private[spark] class AppStatusStore

[GitHub] spark pull request #22683: [SPARK-25696] The storage memory displayed on spa...

2018-12-02 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/22683#discussion_r238102080 --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala --- @@ -1164,17 +1164,17 @@ private[spark] object Utils extends Logging { } else

[GitHub] spark issue #23164: [SPARK-26198][SQL] Fix Metadata serialize null values th...

2018-12-02 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23164 I just mean, is this a bug that comes up otherwise in Spark? should this be back-ported or is it just supporting the new change you reference? I can merge to master at least

[GitHub] spark issue #23150: [SPARK-26178][SQL] Use java.time API for parsing timesta...

2018-12-01 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23150 It makes sense that parsing depends on a timezone, though that's set as an option in the parser typically. The tests should generally test "GMT" for this reason. If there's a default

[GitHub] spark pull request #23173: [SPARK-26208][SQL] add headers to empty csv files...

2018-12-01 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23173#discussion_r238073218 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVFileFormat.scala --- @@ -171,15 +171,21 @@ private[csv] class

[GitHub] spark issue #23162: [MINOR][DOC] Correct some document description errors

2018-12-01 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23162 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #23178: [SPARK-26216][SQL] Do not use case class as publi...

2018-12-01 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23178#discussion_r238062995 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala --- @@ -38,114 +38,106 @@ import

[GitHub] spark issue #23177: [SPARK-26212][Build][test-maven] Upgrade maven version t...

2018-12-01 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23177 Merging to master. I've been using 3.6.0 on the command line for a while and it's fine. Note that if you use your local mvn in IntelliJ, it seems to have some incompatibility with the current latest

[GitHub] spark issue #23185: [MINOR][Docs] Fix typos

2018-11-30 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23185 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #23146: [SPARK-26173] [MLlib] Prior regularization for Lo...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23146#discussion_r237888294 --- Diff: mllib/src/test/scala/org/apache/spark/ml/optim/loss/DifferentiableRegularizationSuite.scala --- @@ -16,9 +16,13 @@ */ package

[GitHub] spark pull request #23146: [SPARK-26173] [MLlib] Prior regularization for Lo...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23146#discussion_r237888645 --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala --- @@ -250,6 +250,66 @@ private[classification] trait

[GitHub] spark pull request #23146: [SPARK-26173] [MLlib] Prior regularization for Lo...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23146#discussion_r237888349 --- Diff: mllib/src/test/scala/org/apache/spark/ml/optim/loss/DifferentiableRegularizationSuite.scala --- @@ -16,9 +16,13 @@ */ package

[GitHub] spark pull request #23146: [SPARK-26173] [MLlib] Prior regularization for Lo...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23146#discussion_r237888585 --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala --- @@ -250,6 +250,66 @@ private[classification] trait

[GitHub] spark pull request #23151: [SPARK-26180][CORE][TEST] Reuse withTempDir funct...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23151#discussion_r237886340 --- Diff: core/src/test/scala/org/apache/spark/SparkFunSuite.scala --- @@ -105,5 +105,16 @@ abstract class SparkFunSuite logInfo(s"

[GitHub] spark pull request #23151: [SPARK-26180][CORE][TEST] Reuse withTempDir funct...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23151#discussion_r237886193 --- Diff: core/src/test/scala/org/apache/spark/SparkFunSuite.scala --- @@ -105,5 +105,16 @@ abstract class SparkFunSuite logInfo(s"

[GitHub] spark pull request #23173: [SPARK-26208][SQL] add headers to empty csv files...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23173#discussion_r237883918 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVFileFormat.scala --- @@ -171,15 +171,21 @@ private[csv] class

[GitHub] spark pull request #23173: [SPARK-26208][SQL] add headers to empty csv files...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23173#discussion_r237884151 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVFileFormat.scala --- @@ -171,15 +171,21 @@ private[csv] class

[GitHub] spark pull request #23177: [SPARK-26212][Build][test-maven] Upgrade maven ve...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23177#discussion_r237881557 --- Diff: pom.xml --- @@ -114,7 +114,7 @@ 1.8 ${java.version} ${java.version} -3.5.4 +3.6.0 --- End diff

[GitHub] spark pull request #23162: [MINOR][DOC] Correct some document description er...

2018-11-30 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23162#discussion_r237880921 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -513,7 +513,7 @@ package object config { "is wr

[GitHub] spark issue #23148: [SPARK-26177] Automated formatting for Scala code

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23148 Yeah we'd need a new PR. If you collect a few good improvements just open a follow up. I was testing by just making a dummy change in a few files and seeing what it did. It's OK as-is, even

[GitHub] spark issue #23177: [SPARK-26212][Build][test-maven] Upgrade maven version t...

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23177 Ah, that's the second time I've forgotten this. Yes looks good to me. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark issue #23126: [SPARK-26158] [MLLIB] fix covariance accuracy problem fo...

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23126 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #23148: [SPARK-26177] Automated formatting for Scala code

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23148 I played with this a little locally, and yeah it does reformat entire files that are in the diff, and most of what it does is fixing stuff we probably wouldn't ask for in a PR review. For example

[GitHub] spark issue #23145: [MINOR][Docs][WIP] Fix Typos

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23145 I think this is fine to merge, this is a good batch of grammar fixes. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark pull request #23145: [MINOR][Docs][WIP] Fix Typos

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23145#discussion_r237564429 --- Diff: docs/structured-streaming-programming-guide.md --- @@ -1634,7 +1634,7 @@ returned through `Dataset.writeStream()`. You will have to specify one

[GitHub] spark pull request #23162: [MINOR][DOC] Correct some document description er...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23162#discussion_r237563687 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -503,7 +503,7 @@ package object config { "made in cre

[GitHub] spark pull request #23162: [MINOR][DOC] Correct some document description er...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23162#discussion_r237563435 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -430,8 +430,8 @@ package object config { .doc("The chunk

[GitHub] spark issue #23148: [SPARK-26177] Automated formatting for Scala code

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23148 Ah I see. I can add that call in a follow-up to enable it and see how we like it. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark issue #23052: [SPARK-26081][SQL] Prevent empty files for empty partiti...

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23052 Merged to master --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #23173: [SPARK-26208][SQL] add headers to empty csv files...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23173#discussion_r237561245 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/OutputWriter.scala --- @@ -57,6 +57,9 @@ abstract class OutputWriterFactory

[GitHub] spark pull request #23151: [SPARK-26180][CORE][TEST] Add a withCreateTempDir...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23151#discussion_r237559695 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala --- @@ -66,6 +66,20 @@ private[sql] trait SQLTestUtils extends SparkFunSuite

[GitHub] spark pull request #23151: [SPARK-26180][CORE][TEST] Add a withCreateTempDir...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23151#discussion_r237559321 --- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala --- @@ -494,13 +494,12 @@ class SparkSubmitSuite } test

[GitHub] spark issue #23177: [SPARK-26212][Build][test-maven] Upgrade maven version t...

2018-11-29 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/23177 That's fine but we need to also update build/mvn to download and use the same version. --- - To unsubscribe, e-mail: reviews

[GitHub] spark pull request #23126: [SPARK-26158] [MLLIB] fix covariance accuracy pro...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23126#discussion_r237556042 --- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/distributed/RowMatrix.scala --- @@ -128,6 +128,82 @@ class RowMatrix @Since("

[GitHub] spark pull request #23126: [SPARK-26158] [MLLIB] fix covariance accuracy pro...

2018-11-29 Thread srowen
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/23126#discussion_r237541497 --- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/distributed/RowMatrix.scala --- @@ -128,6 +128,82 @@ class RowMatrix @Since("

  1   2   3   4   5   6   7   8   9   10   >