[GitHub] spark pull request #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/14915 --- 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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77577909
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -617,7 +617,9 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
case s: String => JString(s)
case u: UUID => JString(u.toString)
case dt: DataType => dt.jsonValue
-case m: Metadata => m.jsonValue
+// SPARK-17356: In usage of mllib, Metadata may store a huge vector of
data, transforming
+// it to JSON may trigger OutOfMemoryError.
+case m: Metadata => Metadata.empty.jsonValue
--- End diff --
oh sorry, I mean `JNull`
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77574630
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -617,7 +617,9 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
case s: String => JString(s)
case u: UUID => JString(u.toString)
case dt: DataType => dt.jsonValue
-case m: Metadata => m.jsonValue
+// SPARK-17356: In usage of mllib, Metadata may store a huge vector of
data, transforming
+// it to JSON may trigger OutOfMemoryError.
+case m: Metadata => Metadata.empty.jsonValue
--- End diff --
No, we should not. JNothing is to map `scala.Option`.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77573016
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -617,7 +617,9 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
case s: String => JString(s)
case u: UUID => JString(u.toString)
case dt: DataType => dt.jsonValue
-case m: Metadata => m.jsonValue
+// SPARK-17356: In usage of mllib, Metadata may store a huge vector of
data, transforming
+// it to JSON may trigger OutOfMemoryError.
+case m: Metadata => Metadata.empty.jsonValue
--- End diff --
shall we use `JNothing` instead of `Metadata.empty.jsonValue`?
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77559438
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -604,6 +604,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
}.toList
}
+ // TODO: Fix toJSON so that we can more safely handle Map and Seq with
loop.
--- End diff --
I removed this comments. I tried, but it seems it requires a big block to
explain what this TODO mean. I feel it may creates bigger confusion.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77441612
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -604,6 +604,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
}.toList
}
+ // TODO: Fix toJSON so that we can more safely handle Map and Seq with
loop.
--- End diff --
I think it is better to make the comment self-contained. So, readers of
this part do not need to guess or search the jira to understand what this line
means.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77301211
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -604,6 +604,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
}.toList
}
+ // TODO: Fix toJSON so that we can more safely handle Map and Seq with
loop.
--- End diff --
I will create a follow up jira to refactor the toJSON
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77288334
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
fixed
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77287956
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- 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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77265668
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
(just make the comment a little bit more clear)
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77265626
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
OK. Let's explain why we do remove the metadata at here. Basically, I think
we need to explain because json string does not have metadata, we remove the
metadata from the normalized plan. So, we can do the check.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77261626
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
@yhuai
It is covered by existing QueryTest. In QueryTest, it will call toJSON and
then call fromJSON to convert JSON back to a plan, and do comparison like this:
```
if (normalized1 != normalized2) {
fail(
s"""
|== FAIL: the logical plan parsed from json does not match the
original one ===
|${sideBySide(logicalPlan.treeString,
normalized2.treeString).mkString("\n")}
""".stripMargin)
}
```
If the Meta data is not empty, the comparison will fail.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77217091
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
It will be good to have a test. The test can check that the json
representation does not have metadata.
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77216938
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -274,6 +274,13 @@ abstract class QueryTest extends PlanTest {
val normalized1 = logicalPlan.transformAllExpressions {
case udf: ScalaUDF => udf.copy(function = null)
case gen: UserDefinedGenerator => gen.copy(function = null)
+ // SPARK-17356: In usage of mllib, Metadata may store a huge vector
of data, transforming
+ // it to JSON may trigger OutOfMemoryError.
+ case a @ Alias(child, name) if a.explicitMetadata.isDefined =>
+Alias(child, name)(a.exprId, a.qualifier, Some(Metadata.empty),
a.isGenerated)
+ case a: AttributeReference if a.metadata != Metadata.empty =>
+AttributeReference(a.name, a.dataType, a.nullable,
Metadata.empty)(a.exprId, a.qualifier,
+ a.isGenerated)
--- End diff --
If we have
https://github.com/apache/spark/pull/14915/files#diff-eac5b02bb450a235fef5e902a2671254R623,
do we still need these?
---
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 #14915: [SPARK-17356][SQL] Fix out of memory issue when g...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/14915#discussion_r77216735
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
---
@@ -604,6 +604,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
}.toList
}
+ // TODO: Fix toJSON so that we can more safely handle Map and Seq with
loop.
--- End diff --
I think this comment deserves to have an example. Also, it will be good to
just create a jira with your example.
---
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
