You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangyum <gi...@git.apache.org> on 2017/05/17 16:03:30 UTC

[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

GitHub user wangyum opened a pull request:

    https://github.com/apache/spark/pull/18019

    [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

    ## What changes were proposed in this pull request?
    Add built-in SQL function `CH[A]R`:
    For `CHR(bigint|double n)`, returns the ASCII character having the binary equivalent to `n`. If n is larger than 256 the result is equivalent to CHR(n % 256)
    
    ## How was this patch tested?
    unit tests


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/wangyum/spark SPARK-20748

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18019.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18019
    
----

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117376928
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    --- End diff --
    
    `defineCodeGen` does not work here. This is a statement instead of an expression.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117623979
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,51 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(lon: Any): Any = {
    +    val longVal = lon.asInstanceOf[Long]
    +    if (longVal < 0) {
    +      UTF8String.EMPTY_UTF8
    +    } else if ((longVal & 0xFF) == 0) {
    +      UTF8String.fromString(Character.MIN_VALUE.toString)
    +    } else {
    +      UTF8String.fromString((longVal & 0xFF).toChar.toString)
    +    }
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    nullSafeCodeGen(ctx, ev, lon => {
    +      s"""
    +        if ($lon < 0) {
    +          ${ev.value} = UTF8String.EMPTY_UTF8;
    +        } else if (($lon & 0xFF) == 0) {
    +          ${ev.value} = UTF8String.fromString(String.valueOf(Character.MIN_VALUE));
    +        } else {
    +          char c = (char)($lon & 0xFF);
    +          ${ev.value} = UTF8String.fromString(String.valueOf(c));
    --- End diff --
    
    It seems difficult to create the bytes directly from char,  `ByteBuffer.allocate(2).putChar(ch).array()` or `new byte[] { (byte)(ch & 0xff), (byte)(ch >> 8 & 0xff) }` not always equals to `String.valueOf(ch).getBytes()`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77015/testReport)** for PR 18019 at commit [`fdd2976`](https://github.com/apache/spark/commit/fdd2976017d575f48c18da95731ba7336e7248c8).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes `


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77371/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117376367
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    --- End diff --
    
    Cast not needed :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77015/testReport)** for PR 18019 at commit [`fdd2976`](https://github.com/apache/spark/commit/fdd2976017d575f48c18da95731ba7336e7248c8).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117378356
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    +        short shortVal;
    +        if (longVal > 255) {
    --- End diff --
    
    It might be easier to something like this:
    ```java
    if (lon < 0) {
      ${ev.value} = UTF8String.fromString(""); // This should be put in a constant
    } else if (($lon & 0xFF) == 0) {
        ${ev.value} = UTF8String.fromString("\u0000"); // This should be put in a variable
    } else {
      char c = (char)($lon & 0xFF);
      // This is quite slow, it might be better to create the bytes directly.
      ${ev.value} = UTF8String.fromString(String.valueOf(c));  
    }
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Thanks! Merging to master.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117289000
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    +        short shortVal;
    +        if (longVal > 255) {
    +          shortVal = (short)(longVal % 256);
    +        } else {
    +          (short)longVal;
    +        }
    +        if (shortVal == 0) {
    +          ${ev.value} = String.valueOf('\u0000');
    --- End diff --
    
    You really should put this in a constant field. It does not make sense to create the same constant over and over...


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Jenkins, retest this please


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117835357
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,51 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    --- End diff --
    
    Also add one more point here. `Invalid code points are not validated.`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77144/testReport)** for PR 18019 at commit [`e003924`](https://github.com/apache/spark/commit/e0039247dd24559d993b7bbc4cd321f9c9198459).
     * 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117378811
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala ---
    @@ -117,6 +117,41 @@ class StringFunctionsSuite extends QueryTest with SharedSQLContext {
           Row(97, 0))
       }
     
    +  test("string ch[a]r function") {
    --- End diff --
    
    Please move this to catalyst. Look at the `StringExpressionsSuite`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77371 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77371/testReport)** for PR 18019 at commit [`ab9e66e`](https://github.com/apache/spark/commit/ab9e66e957614906ceaf6bb85a5d8709feba9f90).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77371 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77371/testReport)** for PR 18019 at commit [`ab9e66e`](https://github.com/apache/spark/commit/ab9e66e957614906ceaf6bb85a5d8709feba9f90).
     * 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77137 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77137/testReport)** for PR 18019 at commit [`e003924`](https://github.com/apache/spark/commit/e0039247dd24559d993b7bbc4cd321f9c9198459).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77137/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77015/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77144/testReport)** for PR 18019 at commit [`e003924`](https://github.com/apache/spark/commit/e0039247dd24559d993b7bbc4cd321f9c9198459).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    LGTM


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77048/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18019


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117375494
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    +        short shortVal;
    +        if (longVal > 255) {
    +          shortVal = (short)(longVal % 256);
    +        } else {
    +          (short)longVal;
    +        }
    +        if (shortVal == 0) {
    +          ${ev.value} = String.valueOf('\u0000');
    +        } else if (shortVal < 0) {
    +          ${ev.value} = "";
    --- End diff --
    
    You cannot assign a `String` to a `UTF8String`. Also make this a constant.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117250008
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    +        short shortVal;
    +        if (longVal > 255) {
    --- End diff --
    
    What about negative values?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117840142
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,51 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    --- End diff --
    
    Nit: Indent issues.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117375988
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, lon => {
    +      s"""
    +        long longVal = (long)${lon};
    +        short shortVal;
    +        if (longVal > 255) {
    +          shortVal = (short)(longVal % 256);
    +        } else {
    +          (short)longVal;
    +        }
    +        if (shortVal == 0) {
    +          ${ev.value} = String.valueOf('\u0000');
    +        } else if (shortVal < 0) {
    +          ${ev.value} = "";
    +        } else {
    +          ${ev.value} = (short)shortVal;
    --- End diff --
    
    You cannot assign a `short` to a `UTF8String`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117378749
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
    @@ -2138,6 +2138,40 @@ object functions {
       def ascii(e: Column): Column = withExpr { Ascii(e.expr) }
     
       /**
    +   * Returns the ASCII character having the binary equivalent to expr.
    --- End diff --
    
    let's not do this just yet; adding it to the function registry is enough.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77048 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77048/testReport)** for PR 18019 at commit [`0fcd9d3`](https://github.com/apache/spark/commit/0fcd9d3a1a7867308c3b8294e0f95607ffbb6d18).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117076510
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(n) - Returns the ASCII character having the binary equivalent to `n`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    +    defineCodeGen(ctx, ev, str => {
    +      s"""
    +        long longVal = ${java.lang.Long.parseLong(new String(str.getBytes))};
    --- End diff --
    
    Isn't the input supposed to be a long?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77144/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18019#discussion_r117250411
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
    @@ -1268,6 +1268,59 @@ case class Ascii(child: Expression) extends UnaryExpression with ImplicitCastInp
     }
     
     /**
    + * Returns the ASCII character having the binary equivalent to n.
    + * If n is larger than 256 the result is equivalent to chr(n % 256)
    + */
    +// scalastyle:off line.size.limit
    +@ExpressionDescription(
    +  usage = "_FUNC_(expr) - Returns the ASCII character having the binary equivalent to `expr`. If n is larger than 256 the result is equivalent to chr(n % 256)",
    +  extended = """
    +    Examples:
    +      > SELECT _FUNC_(65);
    +       A
    +             """)
    +// scalastyle:on line.size.limit
    +case class Chr(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
    +
    +  override def dataType: DataType = StringType
    +  override def inputTypes: Seq[DataType] = Seq(LongType)
    +
    +  protected override def nullSafeEval(value: Any): Any = {
    +    val longVal = value.asInstanceOf[Long]
    +    val shortVal = if (longVal > 255) (longVal % 256).toShort else longVal.toShort
    +    val stringVal = if (shortVal == 0) {
    +      String.valueOf('\u0000')
    +    } else if (shortVal < 0) {
    +      ""
    +    } else {
    +      shortVal.toChar.toString
    +    }
    +    UTF8String.fromString(stringVal)
    +  }
    +
    +  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    --- End diff --
    
    I am 100% this is not being tested. Can you check how other expression test the code they generate?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18019: [SPARK-20748][SQL] Add built-in SQL function CH[A]R.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18019
  
    **[Test build #77048 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77048/testReport)** for PR 18019 at commit [`0fcd9d3`](https://github.com/apache/spark/commit/0fcd9d3a1a7867308c3b8294e0f95607ffbb6d18).
     * 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org