[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/15994 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r90635187
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
@windpiger I don't think this was quite resolved -- you need to back-tick
DataFrame unfortunately to get it to work with javadoc 8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r90167716
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +438,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new `DataFrame` that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
--- End diff --
@srowen
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89924211
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +438,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new `DataFrame` that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
--- End diff --
what about change `fill1` to `fillValue` ,and change `private def
fill0(values: Seq[(String, Any)])` to `private def fillMap(values: Seq[(String,
Any)])` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89772926
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -145,6 +151,14 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
* Returns a new [[DataFrame]] that replaces null or NaN values in
specified numeric columns.
* If a specified column is not a numeric column, it is ignored.
*
+ * @since 2.1.0
--- End diff --
I think this may be in 2.2.0 in the end
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89772231
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +438,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new `DataFrame` that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
--- End diff --
"fill1" isn't a good name. "fillInternal" maybe?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89747135
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
@HyukjinKwon yeah the bad news is that I'm sure the javadoc generation is
going to re-break periodically. we can try to catch it with reviews and your
work at least gets it to a working state. But we'll clean it up again before
releases regularly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89742322
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+// the fill[T] which T is Long/Integer/Float/Double,
+// should apply on all the NumericType Column, for example:
+// val input = Seq[(java.lang.Integer, java.lang.Double)]((null,
164.3)).toDF("a","b")
+// input.na.fill(3.1)
+// the result is (3,164.3), not (null, 164.3)
+val targetType = value match {
+ case _: jl.Double | _: jl.Integer | _: jl.Float | _: jl.Long =>
NumericType
--- End diff --
fixed it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89742186
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
ok
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89741302
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
What if that one is merged first?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89740897
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
change them in #16013 is better?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89740299
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+// the fill[T] which T is Long/Integer/Float/Double,
--- End diff --
remove them is ok
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89737784
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -130,6 +130,13 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
/**
* Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long): DataFrame = fill(value, df.columns)
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
--- End diff --
Could I ask change `[[DataFrame]]` to `` `DataFrame` ``? It seems the
`DataFrame` is unrecognisable via unidoc/genjavadoc (see
https://github.com/apache/spark/pull/16013) which ends up with documentation
build failure with Java 8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89737081
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+// the fill[T] which T is Long/Integer/Float/Double,
+// should apply on all the NumericType Column, for example:
+// val input = Seq[(java.lang.Integer, java.lang.Double)]((null,
164.3)).toDF("a","b")
+// input.na.fill(3.1)
+// the result is (3,164.3), not (null, 164.3)
+val targetType = value match {
+ case _: jl.Double | _: jl.Integer | _: jl.Float | _: jl.Long =>
NumericType
--- End diff --
why we match `jd.Double` here intead of scala `Double`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89736984
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+// the fill[T] which T is Long/Integer/Float/Double,
+// should apply on all the NumericType Column, for example:
+// val input = Seq[(java.lang.Integer, java.lang.Double)]((null,
164.3)).toDF("a","b")
+// input.na.fill(3.1)
+// the result is (3,164.3), not (null, 164.3)
--- End diff --
`(3, 164.3)`? shouldn't it be 3.1?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89736915
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * Returns a new [[DataFrame]] that replaces null or NaN values in
specified
+ * numeric, string columns. If a specified column is not a numeric,
string column,
+ * it is ignored.
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+// the fill[T] which T is Long/Integer/Float/Double,
--- End diff --
why the T can be `Integer` and `Float`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89736733
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -153,19 +168,20 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
* (Scala-specific) Returns a new [[DataFrame]] that replaces null or
NaN values in specified
* numeric columns. If a specified column is not a numeric column, it is
ignored.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long, cols: Seq[String]): DataFrame = {
+fill1(value, cols)
--- End diff --
nit: put it in one line? i.e. `def fill... = fill1...`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89727312
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * (Scala-specific) Returns a new [[DataFrame]] that replaces null or
NaN values in specified
+ * numeric, bool, string columns. If a specified column is not a
numeric, boolean, string column,
+ * it is ignored.
+ *
+ * @since 2.1.0
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+value match {
+ case _: jl.Double | _: jl.Integer | _: jl.Float | _: jl.Boolean | _:
jl.Long | _: String =>
+ case _ =>
+throw new IllegalArgumentException(
+ s"Unsupported value type ${value.getClass.getName} ($value).")
+}
+
+val targetColumnType = value match {
+ case _: jl.Double | _: jl.Integer | _: jl.Float | _: jl.Long =>
NumericType
+ case _: jl.Boolean => BooleanType
+ case _: String => StringType
+}
+
+val columnEquals = df.sparkSession.sessionState.analyzer.resolver
+val projections = df.schema.fields.map { f =>
+ // Only fill if the column is part of the cols list.
+ if (((f.dataType.isInstanceOf[NumericType] && targetColumnType ==
NumericType)
--- End diff --
Thanks!
I have modified except one:
If T is a double type , this should be apply to all Numeric columns(include
LongType/IntegerType), or just apply to FractionType?
The fill(value Double) apply to all Numeric columns, and I think fill(value
Long) also keep the logic.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89721405
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * (Scala-specific) Returns a new [[DataFrame]] that replaces null or
NaN values in specified
+ * numeric, bool, string columns. If a specified column is not a
numeric, boolean, string column,
+ * it is ignored.
+ *
+ * @since 2.1.0
+ */
+ private def fill1[T](value: T, cols: Seq[String]): DataFrame = {
+value match {
+ case _: jl.Double | _: jl.Integer | _: jl.Float | _: jl.Boolean | _:
jl.Long | _: String =>
+ case _ =>
+throw new IllegalArgumentException(
+ s"Unsupported value type ${value.getClass.getName} ($value).")
+}
+
+val targetColumnType = value match {
--- End diff --
nit: we can combine the check here:
```
val targetType = value match {
case _: Long => LongType
case _: Double => DoubleType
case _: String => StringType
case _ => throw ...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89721142
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -437,4 +444,38 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
case v => throw new IllegalArgumentException(
s"Unsupported value type ${v.getClass.getName} ($v).")
}
+
+ /**
+ * (Scala-specific) Returns a new [[DataFrame]] that replaces null or
NaN values in specified
--- End diff --
we don't need `(Scala-specific)` and the `since` tag for private methods.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89721112
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -153,19 +168,20 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
* (Scala-specific) Returns a new [[DataFrame]] that replaces null or
NaN values in specified
* numeric columns. If a specified column is not a numeric column, it is
ignored.
*
+ * @since 2.1.0
+ */
+ def fill(value: Long, cols: Seq[String]): DataFrame = {
+fill1[Long](value, cols)
--- End diff --
nit: `fill1(value, cols)` should work, scala has type inference.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89721087
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -128,66 +128,49 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
}
/**
- * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
+ * Returns a new [[DataFrame]] that replaces null or NaN values
+ * in numeric, boolean, string columns with `value`.
*
* @since 1.3.1
*/
- def fill(value: Double): DataFrame = fill(value, df.columns)
+ def fill[T](value: T): DataFrame = fill(value, df.columns)
--- End diff --
ok, thanks a lot ! I have put it as a private. @cloud-fan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89690706
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -128,66 +128,49 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
}
/**
- * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
+ * Returns a new [[DataFrame]] that replaces null or NaN values
+ * in numeric, boolean, string columns with `value`.
*
* @since 1.3.1
*/
- def fill(value: Double): DataFrame = fill(value, df.columns)
+ def fill[T](value: T): DataFrame = fill(value, df.columns)
--- End diff --
I think we can have a `private def fill[T]`, but the public API can't be
changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89687890
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -128,66 +128,49 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
}
/**
- * Returns a new [[DataFrame]] that replaces null or NaN values in
numeric columns with `value`.
+ * Returns a new [[DataFrame]] that replaces null or NaN values
+ * in numeric, boolean, string columns with `value`.
*
* @since 1.3.1
*/
- def fill(value: Double): DataFrame = fill(value, df.columns)
+ def fill[T](value: T): DataFrame = fill(value, df.columns)
--- End diff --
add a long type function is ok. I thought the string/long/double logic is
the same ,so I want to merge them by a template function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89662083
--- Diff: project/MimaExcludes.scala ---
@@ -529,6 +529,7 @@ object MimaExcludes {
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.mllib.evaluation.MulticlassMetrics.this"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.mllib.evaluation.RegressionMetrics.this"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.sql.DataFrameNaFunctions.this"),
+
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.sql.DataFrameNaFunctions.fill"),
--- End diff --
The thing is they are not backward compatible at bytecode level, so
applications will break if they are not rebuilt.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89331384
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -132,62 +132,42 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
*
* @since 1.3.1
*/
- def fill(value: Double): DataFrame = fill(value, df.columns)
-
- /**
- * Returns a new [[DataFrame]] that replaces null values in string
columns with `value`.
- *
- * @since 1.3.1
- */
- def fill(value: String): DataFrame = fill(value, df.columns)
--- End diff --
Doesn't it change the binary signature of the API? That is the problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
Github user windpiger commented on a diff in the pull request:
https://github.com/apache/spark/pull/15994#discussion_r89326882
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/DataFrameNaFunctions.scala ---
@@ -132,62 +132,42 @@ final class DataFrameNaFunctions private[sql](df:
DataFrame) {
*
* @since 1.3.1
*/
- def fill(value: Double): DataFrame = fill(value, df.columns)
-
- /**
- * Returns a new [[DataFrame]] that replaces null values in string
columns with `value`.
- *
- * @since 1.3.1
- */
- def fill(value: String): DataFrame = fill(value, df.columns)
--- End diff --
I thought fill[T](value: T) can handle the String Type, then I remove
it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #15994: [SPARK-18555][SQL]DataFrameNaFunctions.fill miss ...
GitHub user windpiger opened a pull request:
https://github.com/apache/spark/pull/15994
[SPARK-18555][SQL]DataFrameNaFunctions.fill miss up original values in long
integers
## What changes were proposed in this pull request?
DataSet.na.fill(0) used on a DataSet which has a long value column, it
will change the original long value. For example:
```
scala> val df = Seq[(Long, Long)]((1, 2), (-1, -2), (9123146099426677101L,
9123146560113991650L)).toDF("a", "b")
df: org.apache.spark.sql.DataFrame = [a: bigint, b: bigint]
scala> df.show
+---+---+
| a| b|
+---+---+
| 1| 2|
| -1| -2|
|9123146099426677101|9123146560113991650|
+---+---+
scala> df.na.fill(0).show
+---+---+
| a| b|
+---+---+
| 1| 2|
| -1| -2|
|9123146099426676736|9123146560113991680|
+---+---+
```
the original values changed [which is not we expected result]:
```
9123146099426677101 -> 9123146099426676736
9123146560113991650 -> 9123146560113991680
```
## How was this patch tested?
unit test added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/windpiger/spark nafillMissupOriginalValue
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15994.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15994
commit 1ecc21e35aa52e6989ae86a92740fe8235de28a9
Author: root
Date: 2016-11-23T13:33:16Z
[SPARK-18555][SQL]DataFrameNaFunctions.fill miss up original values in long
integers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
