[GitHub] spark pull request: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221330231 `DefinedByConstructorParams` is a good idea --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221172065 Can we just use DefinedByConstructorParams rather than using case classes? --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64327921
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,39 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.rdd.count
+ }
}
+
+case class S100(
--- End diff --
scala 2.10 doesn't support large case class. We can create a new test suite
under `scala-2.11/src/test` and put this test there, so that we only run it
under scala 2.10. `repl` module is a good example about it. cc @kiszk do you
mind resend this PR with the fix?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221165640 Sorry we have to revert it as it breaks scala-2.10 build --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221162353 thanks, merging to master and 2.0! --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/13243 --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221158001 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221158003 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59173/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221157890 **[Test build #59173 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59173/consoleFull)** for PR 13243 at commit [`b92ab8c`](https://github.com/apache/spark/commit/b92ab8cb8fd4d9aabda7486cf9d2d5dd7b98f0bf). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221147028 **[Test build #59173 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59173/consoleFull)** for PR 13243 at commit [`b92ab8c`](https://github.com/apache/spark/commit/b92ab8cb8fd4d9aabda7486cf9d2d5dd7b98f0bf). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64318271
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,38 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.show
--- End diff --
Again, I conformed that to call ```collect``` cannot reproduce this bug in
the latest master branch. However, I confirmed that to call
```ds.rdd.collect``` can reproduce this bug. The latest code uses
```ds.rdd.collect```.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221083826 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59149/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221083825 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221083534 **[Test build #59149 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59149/consoleFull)** for PR 13243 at commit [`6f5bda3`](https://github.com/apache/spark/commit/6f5bda30d2e24347501f02881132a734b0ed1d7c). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64277601
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,38 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.show
--- End diff --
hmm, this is weird, the main logic in `show` is calling `collect` to get
the data, can you double check it? and how about `ds.rdd.collect`?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-221058608 **[Test build #59149 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59149/consoleFull)** for PR 13243 at commit [`6f5bda3`](https://github.com/apache/spark/commit/6f5bda30d2e24347501f02881132a734b0ed1d7c). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64266488
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,38 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.show
--- End diff --
To use ``collect`` cannot reproduce this bug. I think that to use ``show``
involves code generation for ```Projection```.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64245614
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,38 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.show
+ }
}
+
+case class S100(
+ s1: String = "1", s2: String = "2", s3: String = "3", s4: String = "4",
+ s5: String = "5", s6: String = "6", s7: String = "7", s8: String = "8",
+ s9: String = "9", s10: String = "10", s11: String = "11", s12: String =
"12",
+ s13: String = "13", s14: String = "14", s15: String = "15", s16: String
= "16",
+ s17: String = "17", s18: String = "18", s19: String = "19", s20: String
= "20",
+ s21: String = "21", s22: String = "22", s23: String = "23", s24: String
= "24",
+ s25: String = "25", s26: String = "26", s27: String = "27", s28: String
= "28",
+ s29: String = "29", s30: String = "30", s31: String = "31", s32: String
= "32",
+ s33: String = "33", s34: String = "34", s35: String = "35", s36: String
= "36",
+ s37: String = "37", s38: String = "38", s39: String = "39", s40: String
= "40",
+ s41: String = "41", s42: String = "42", s43: String = "43", s44: String
= "44",
+ s45: String = "45", s46: String = "46", s47: String = "47", s48: String
= "48",
+ s49: String = "49", s50: String = "50", s51: String = "51", s52: String
= "52",
+ s53: String = "53", s54: String = "54", s55: String = "55", s56: String
= "56",
+ s57: String = "57", s58: String = "58", s59: String = "59", s60: String
= "60",
+ s61: String = "61", s62: String = "62", s63: String = "63", s64: String
= "64",
+ s65: String = "65", s66: String = "66", s67: String = "67", s68: String
= "68",
+ s69: String = "69", s70: String = "70", s71: String = "71", s72: String
= "72",
+ s73: String = "73", s74: String = "74", s75: String = "75", s76: String
= "76",
+ s77: String = "77", s78: String = "78", s79: String = "79", s80: String
= "80",
+ s81: String = "81", s82: String = "82", s83: String = "83", s84: String
= "84",
+ s85: String = "85", s86: String = "86", s87: String = "87", s88: String
= "88",
+ s89: String = "89", s90: String = "90", s91: String = "91", s92: String
= "92",
+ s93: String = "93", s94: String = "94", s95: String = "95", s96: String
= "96",
+ s97: String = "97", s98: String = "98", s99: String = "99", s100: String
= "100")
+case class S100_5(
--- End diff --
add a blank line between these 2 classes.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64245550
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -58,4 +58,38 @@ class DataFrameComplexTypeSuite extends QueryTest with
SharedSQLContext {
val nullIntRow = df.selectExpr("i[1]").collect()(0)
assert(nullIntRow == org.apache.spark.sql.Row(null))
}
+
+ test("SPARK-15285 Generated SpecificSafeProjection.apply method grows
beyond 64KB") {
+val ds100_5 = Seq(S100_5()).toDS()
+ds100_5.show
--- End diff --
we should not use `show` in test, can `collect` reproduce this bug?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220952732 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59126/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220952730 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220952549 **[Test build #59126 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59126/consoleFull)** for PR 13243 at commit [`6e730ca`](https://github.com/apache/spark/commit/6e730ca6215afca78c0d3d34bd07218962475aaa). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64196573
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,47 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls,
+ s"$argIsNulls = new boolean[${arguments.size}];")
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
--- End diff --
I was afraid of increasing code size. However, in the case of 1 or 2
arguments, the code size of generating argument value are relatively small than
the cases with large number of arguments. As a result, code size here would not
be an issue.
Thus, the latest version always uses for-loop.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64196297
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,47 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls,
+ s"$argIsNulls = new boolean[${arguments.size}];")
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
+ if ($argIsNulls[idx]) { $isNull = true; break; }
+ }
+ """
} else {
isNull = "false"
""
}
val constructorCall = outer.map { gen =>
- s"""${gen.value}.new ${cls.getSimpleName}($argString)"""
+ s"""${gen.value}.new ${cls.getSimpleName}(${argValues.mkString(",
")})"""
}.getOrElse {
- s"new $className($argString)"
+ s"new $className(${argValues.mkString(", ")})"
}
val code = s"""
- ${argGen.map(_.code).mkString("\n")}
+ ${argCode}
--- End diff --
Good catch, thanks.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220935411 **[Test build #59126 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59126/consoleFull)** for PR 13243 at commit [`6e730ca`](https://github.com/apache/spark/commit/6e730ca6215afca78c0d3d34bd07218962475aaa). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220902483 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220902485 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59122/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220902329 **[Test build #59122 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59122/consoleFull)** for PR 13243 at commit [`9cc4d41`](https://github.com/apache/spark/commit/9cc4d41ecb69c0b6fecf11da5683f1114d418e4d). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64172891
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,47 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls,
+ s"$argIsNulls = new boolean[${arguments.size}];")
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
+ if ($argIsNulls[idx]) { $isNull = true; break; }
+ }
+ """
} else {
isNull = "false"
""
}
val constructorCall = outer.map { gen =>
- s"""${gen.value}.new ${cls.getSimpleName}($argString)"""
+ s"""${gen.value}.new ${cls.getSimpleName}(${argValues.mkString(",
")})"""
}.getOrElse {
- s"new $className($argString)"
+ s"new $className(${argValues.mkString(", ")})"
}
val code = s"""
- ${argGen.map(_.code).mkString("\n")}
+ ${argCode}
--- End diff --
nit: `$argCode`, no need of brace.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64172857
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,47 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls,
+ s"$argIsNulls = new boolean[${arguments.size}];")
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
--- End diff --
Do you have some benchmark number for it? Especially for 1 or 2 arguments.
If there is a lot of improvement, we can specialize the code when there are 1
or 2 arguments.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220890649 **[Test build #59122 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59122/consoleFull)** for PR 13243 at commit [`9cc4d41`](https://github.com/apache/spark/commit/9cc4d41ecb69c0b6fecf11da5683f1114d418e4d). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220883772 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220883773 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59118/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220883664 **[Test build #59118 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59118/consoleFull)** for PR 13243 at commit [`74d8764`](https://github.com/apache/spark/commit/74d876453cf853a97884ff195616e909e5873a2c). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64166173
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,54 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls,
+ s"$argIsNulls = new boolean[${arguments.size}];")
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
+val argIsNull = arguments.zipWithIndex.map { case (e, i) =>
+ s"$argIsNulls[$i]"
+}
+s"final boolean $isNull = ${argIsNull.mkString(" || ")};"
+ } else {
+s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
+ if ($argIsNulls[idx]) { $isNull = true; break; }
+ }
+ """
+ }
} else {
isNull = "false"
""
}
val constructorCall = outer.map { gen =>
- s"""${gen.value}.new ${cls.getSimpleName}($argString)"""
+ s"""${gen.value}.new ${cls.getSimpleName}(${argValues.mkString(",
")})"""
}.getOrElse {
- s"new $className($argString)"
+ s"new $className(${argValues.mkString(", ")})"
}
val code = s"""
- ${argGen.map(_.code).mkString("\n")}
+ ${argCode.mkString("")}
--- End diff --
Isn't `argCode` already a string?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64164066
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
--- End diff --
Sure. But, do we use the for-loop version for 1 or 2? For 1, clearly, we do
not need a loop. For 2, we always exit a loop only for executing one iteration.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220876025 **[Test build #59118 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59118/consoleFull)** for PR 13243 at commit [`74d8764`](https://github.com/apache/spark/commit/74d876453cf853a97884ff195616e909e5873a2c). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64163700
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
--- End diff --
Sure, I did.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64162830
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
--- End diff --
BTW how useful is this optimization? What if we always use the for-loop
version? It will be better if we can have a small benchmark here.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64162262
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
--- End diff --
I have no particular reason for pick ```10```. ```100``` seems to be too
long.
Do you have any suggestion to pick the value?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220844617 mostly LGTM except some minor comments, thanks for working on 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64153183
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
+val argIsNull = arguments.zipWithIndex.map { case (e, i) =>
+ s"$argIsNulls[$i]"
+}
+s"final boolean $isNull = ${argIsNull.mkString(" || ")};"
+ } else {
+s"""
+ boolean $isNull = false;
+ for (int idx = 0; idx < ${arguments.length}; idx++) {
+ if ($argIsNulls[idx]) { $isNull = true; break; }
+ }
+ """
+ }
} else {
isNull = "false"
""
}
val constructorCall = outer.map { gen =>
- s"""${gen.value}.new ${cls.getSimpleName}($argString)"""
+ s"""${gen.value}.new ${cls.getSimpleName}(${argValues.mkString(",
")})"""
}.getOrElse {
- s"new $className($argString)"
+ s"new $className(${argValues.mkString(", ")})"
}
val code = s"""
- ${argGen.map(_.code).mkString("\n")}
+ $argIsNulls = new boolean[${arguments.size}];
--- End diff --
do we need to create a new boolean array every round?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64153170
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
+ argValue
+}
+
+val argCodes = arguments.zipWithIndex.map { case (e, i) =>
+ val expr = e.genCode(ctx)
+ expr.code + s"""
+ $argIsNulls[$i] = ${expr.isNull};
+ ${argValues(i)} = ${expr.value};
+ """
+}
+val argCode = ctx.splitExpressions(ctx.INPUT_ROW, argCodes)
val outer = outerPointer.map(func =>
Literal.fromObject(func()).genCode(ctx))
var isNull = ev.isNull
val setIsNull = if (propagateNull && arguments.nonEmpty) {
- s"final boolean $isNull = ${argGen.map(_.isNull).mkString(" || ")};"
+ if (arguments.length <= 10) {
--- End diff --
any particular reason to pick `10`?
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13243#discussion_r64153124
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -232,27 +232,55 @@ case class NewInstance(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val javaType = ctx.javaType(dataType)
-val argGen = arguments.map(_.genCode(ctx))
-val argString = argGen.map(_.value).mkString(", ")
+val argIsNulls = ctx.freshName("argIsNulls")
+ctx.addMutableState("boolean[]", argIsNulls, "")
+val argTypes = arguments.map(e => ctx.javaType(e.dataType))
+val argValues = arguments.zipWithIndex.map { case (e, i) =>
+ val argValue = ctx.freshName("argValue")
+ ctx.addMutableState(argTypes(i), argValue, "")
--- End diff --
nit: `ctx.addMutableState(ctx.javaType(e.dataType), argValue, "")`, then we
don't need to define `argTypes`.
---
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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220840617 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220840619 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59109/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220840523 **[Test build #59109 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59109/consoleFull)** for PR 13243 at commit [`5bdfaa7`](https://github.com/apache/spark/commit/5bdfaa73f8d458b9994dd1a11b535d9413674484). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220835818 **[Test build #59109 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59109/consoleFull)** for PR 13243 at commit [`5bdfaa7`](https://github.com/apache/spark/commit/5bdfaa73f8d458b9994dd1a11b535d9413674484). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220833378 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59098/ Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220833377 Merged build finished. Test PASSed. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220833313 **[Test build #59098 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59098/consoleFull)** for PR 13243 at commit [`7c35c12`](https://github.com/apache/spark/commit/7c35c12d7bf0709d1dfc1f62167aa5a82a1ac2a6). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user kiszk commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220824933 Let me see the approach, which you proposed, later. --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220823691 **[Test build #59098 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59098/consoleFull)** for PR 13243 at commit [`7c35c12`](https://github.com/apache/spark/commit/7c35c12d7bf0709d1dfc1f62167aa5a82a1ac2a6). --- 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: [SPARK-15285][SQL] Generated SpecificSafeProje...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/13243#issuecomment-220815424 The fallback approach doesn't look that simple and clean, can you try split the generated code like we did in `CreateExternalRow`? --- 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
