[SPARK-8248][SQL] string function: length #6724

chenghao-intel · 2015-06-09T16:11:38Z

No description provided.

chenghao-intel · 2015-06-09T16:35:10Z

@rxin can you review this? Once this merged, the other string functions can take this as the example.

SparkQA · 2015-06-09T16:35:28Z

Test build #34512 has finished for PR 6724 at commit 548d2ef.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleFunctionRegistry extends FunctionRegistry
- case class Rand(seed: Long) extends RDG(seed)
- case class Randn(seed: Long) extends RDG(seed)
- case class Length(child: Expression) extends UnaryExpression with ExpectsInputTypes
- class StringKeyHashMap[T](normalizer: (String) => String)
- logInfo(s"Using user defined output committer class $
- logInfo(s"Using output committer class $

SparkQA · 2015-06-09T16:51:51Z

Test build #34513 has finished for PR 6724 at commit db604ae.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Length(child: Expression) extends UnaryExpression with ExpectsInputTypes

rxin · 2015-06-09T20:45:38Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+    expression[Lower]("lower"),
+    expression[Substring]("substr"),
+    expression[Substring]("substring"),
+    expression[Length]("length")


can you sort this alphabetically

davies · 2015-06-09T23:54:51Z

For failed tests, it will be fixed by c8e7cd2

davies · 2015-06-09T23:55:51Z

How can we support get the length of BinaryType, ArrayType and MapType?

rxin · 2015-06-09T23:56:44Z

They are different functions right now. We can still support them even if they are named the same though.

rxin · 2015-06-10T02:16:02Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+    // string functions
+    expression[Upper]("lcase"),
+    expression[Lower]("lower"),
+    expression[StringLength]("strlen"),


don't rename this one since we need hive compatibility here... only rename the data frame function.

SparkQA · 2015-06-10T02:24:15Z

Test build #34554 has finished for PR 6724 at commit 8e30171.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

chenghao-intel · 2015-06-10T02:39:31Z

...catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringFunctionsSuite.scala

+    checkEvaluation(StringLength(regEx), 5, create_row("abdef"))
+    checkEvaluation(StringLength(regEx), 0, create_row(""))
+    checkEvaluation(StringLength(regEx), null, create_row(null))
+    checkEvaluation(StringLength(Literal.create(null, StringType)), null, create_row("abdef"))


As @davies pointed out, this probably failed in codegen.

can you pull in his fix?

OK, I thought @davies will fix this. I will take look at this.

Yea but his fix won't be merged for a while because it's part of a much broader change.

I can port some of fix from that big PR as a separate PR.

Let's do that. Take your big PR into smaller ones.

SparkQA · 2015-06-10T02:58:49Z

Test build #34555 has finished for PR 6724 at commit 3641f06.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

SparkQA · 2015-06-10T06:55:03Z

Test build #34561 has finished for PR 6724 at commit 3c729aa.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

rxin · 2015-06-10T06:57:11Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala

-      }
-    """
+    child match {
+      case Literal(null, _) =>


@davies can you review this part?

ping @davies

I'd like to fix this in Literal.genCode, or you have do fix it in many places, for example, BinaryExpression also should fix it.

Without this change, it will throws exception

Code generation of strlen(null) failed: [info] [info] int primitive1 = -1; [info] if (!true) { [info] primitive1 = (null).length(); [info] }

Ok, I will leave this for you @davies, and disable the null test temporally in the unit test.

rxin · 2015-06-10T07:02:09Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

@@ -1299,6 +1300,19 @@ object functions {
   */
  def toRadians(columnName: String): Column = toRadians(Column(columnName))

+  /**
+   * Length of a given string value


Computes the length of a string column.

SparkQA · 2015-06-10T07:13:23Z

Test build #34572 has finished for PR 6724 at commit 3e92d32.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

SparkQA · 2015-06-10T09:48:57Z

Test build #34584 has finished for PR 6724 at commit 1eb1fd1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

chenghao-intel · 2015-06-10T11:03:23Z

retest this please

SparkQA · 2015-06-10T15:04:04Z

Test build #34591 has finished for PR 6724 at commit 1eb1fd1.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

rxin · 2015-06-10T16:52:47Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

@@ -37,6 +37,7 @@ import org.apache.spark.util.Utils
 * @groupname normal_funcs Non-aggregate functions
 * @groupname math_funcs Math functions
 * @groupname window_funcs Window functions
+ * @groupname string_funcs functions for DataFrames.


"functions for DataFrames " -> "String functions"

SparkQA · 2015-06-11T02:16:45Z

Test build #34645 has finished for PR 6724 at commit ae08003.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

chenghao-intel · 2015-06-11T02:39:11Z

@rxin, any more comment before merge?

chenghao-intel · 2015-06-11T02:41:25Z

I will create another PR with bunch of string functions after this merged, it will be great if this goes first.

SparkQA · 2015-06-11T02:48:43Z

Test build #34647 has finished for PR 6724 at commit aaa3c31.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes

rxin · 2015-06-11T02:53:43Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+  //////////////////////////////////////////////////////////////////////////////////////////////
+
+  /**
+   * Computes the length of a given string value


in your next pr, please add a period to the end of this.

rxin · 2015-06-11T02:55:14Z

OK I'm merging this one.

Author: Cheng Hao <hao.cheng@intel.com> Closes apache#6724 from chenghao-intel/length and squashes the following commits: aaa3c31 [Cheng Hao] revert the additional change 97148a9 [Cheng Hao] remove the codegen testing temporally ae08003 [Cheng Hao] update the comments 1eb1fd1 [Cheng Hao] simplify the code as commented 3e92d32 [Cheng Hao] use the selectExpr in unit test intead of SQLQuery 3c729aa [Cheng Hao] fix bug for constant null value in codegen 3641f06 [Cheng Hao] keep the length() method for registered function 8e30171 [Cheng Hao] update the code as comment db604ae [Cheng Hao] Add code gen support 548d2ef [Cheng Hao] register the length() 09a0738 [Cheng Hao] add length support

chenghao-intel added 3 commits June 10, 2015 00:03

add length support

09a0738

register the length()

548d2ef

Add code gen support

db604ae

rxin reviewed Jun 9, 2015
View reviewed changes

update the code as comment

8e30171

rxin reviewed Jun 10, 2015
View reviewed changes

keep the length() method for registered function

3641f06

chenghao-intel reviewed Jun 10, 2015
View reviewed changes

fix bug for constant null value in codegen

3c729aa

chenghao-intel mentioned this pull request Jun 10, 2015

[SPARK-8217] [SQL] math function log2 #6718

Closed

rxin reviewed Jun 10, 2015
View reviewed changes

use the selectExpr in unit test intead of SQLQuery

3e92d32

rxin reviewed Jun 10, 2015
View reviewed changes

simplify the code as commented

1eb1fd1

rxin reviewed Jun 10, 2015
View reviewed changes

chenghao-intel added 3 commits June 11, 2015 08:23

update the comments

ae08003

remove the codegen testing temporally

97148a9

revert the additional change

aaa3c31

rxin reviewed Jun 11, 2015
View reviewed changes

asfgit closed this in 9fe3adc Jun 11, 2015

chenghao-intel deleted the length branch July 2, 2015 08:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-8248][SQL] string function: length #6724

[SPARK-8248][SQL] string function: length #6724

chenghao-intel commented Jun 9, 2015

chenghao-intel commented Jun 9, 2015

SparkQA commented Jun 9, 2015

SparkQA commented Jun 9, 2015

rxin Jun 9, 2015

davies commented Jun 9, 2015

davies commented Jun 9, 2015

rxin commented Jun 9, 2015

rxin Jun 10, 2015

SparkQA commented Jun 10, 2015

chenghao-intel Jun 10, 2015

rxin Jun 10, 2015

chenghao-intel Jun 10, 2015

rxin Jun 10, 2015

davies Jun 11, 2015

rxin Jun 11, 2015

SparkQA commented Jun 10, 2015

SparkQA commented Jun 10, 2015

rxin Jun 10, 2015

chenghao-intel Jun 11, 2015

davies Jun 11, 2015

chenghao-intel Jun 11, 2015

chenghao-intel Jun 11, 2015

rxin Jun 10, 2015

SparkQA commented Jun 10, 2015

SparkQA commented Jun 10, 2015

chenghao-intel commented Jun 10, 2015

SparkQA commented Jun 10, 2015

rxin Jun 10, 2015

SparkQA commented Jun 11, 2015

chenghao-intel commented Jun 11, 2015

chenghao-intel commented Jun 11, 2015

SparkQA commented Jun 11, 2015

rxin Jun 11, 2015

rxin commented Jun 11, 2015

[SPARK-8248][SQL] string function: length #6724

[SPARK-8248][SQL] string function: length #6724

Conversation

chenghao-intel commented Jun 9, 2015

chenghao-intel commented Jun 9, 2015

SparkQA commented Jun 9, 2015

SparkQA commented Jun 9, 2015

Choose a reason for hiding this comment

davies commented Jun 9, 2015

davies commented Jun 9, 2015

rxin commented Jun 9, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jun 10, 2015

SparkQA commented Jun 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jun 10, 2015

SparkQA commented Jun 10, 2015

chenghao-intel commented Jun 10, 2015

SparkQA commented Jun 10, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 11, 2015

chenghao-intel commented Jun 11, 2015

chenghao-intel commented Jun 11, 2015

SparkQA commented Jun 11, 2015

Choose a reason for hiding this comment

rxin commented Jun 11, 2015