Skip to content

Commit 79846bf

Browse files
committed
address comments
1 parent 6da0f8f commit 79846bf

File tree

3 files changed

+37
-42
lines changed

3 files changed

+37
-42
lines changed

common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,7 @@ public UTF8String trimLeft() {
544544
* @param trimString the trim character string
545545
*/
546546
public UTF8String trimLeft(UTF8String trimString) {
547-
if (trimString == null)
548-
return null;
547+
if (trimString == null) return null;
549548
// the searching byte position in the source string
550549
int srchIdx = 0;
551550
// the first beginning byte position of a non-matching character
@@ -592,8 +591,7 @@ public UTF8String trimRight() {
592591
* @param trimString the trim character string
593592
*/
594593
public UTF8String trimRight(UTF8String trimString) {
595-
if (trimString == null)
596-
return null;
594+
if (trimString == null) return null;
597595
int charIdx = 0;
598596
// number of characters from the source string
599597
int numChars = 0;
@@ -604,7 +602,7 @@ public UTF8String trimRight(UTF8String trimString) {
604602
// build the position and length array
605603
while (charIdx < numBytes) {
606604
stringCharPos[numChars] = charIdx;
607-
stringCharLen[numChars]= numBytesForFirstByte(getByte(charIdx));
605+
stringCharLen[numChars] = numBytesForFirstByte(getByte(charIdx));
608606
charIdx += stringCharLen[numChars];
609607
numChars ++;
610608
}

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

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ object StringTrim {
519519
}
520520

521521
/**
522-
* A function that takes a character string, removes the leading and trailing characters matching with the characters
522+
* A function that takes a character string, removes the leading and trailing characters matching with any character
523523
* in the trim string, returns the new string.
524524
* If BOTH and trimStr keywords are not specified, it defaults to remove space character from both ends. The trim
525525
* function will have one argument, which contains the source string.
@@ -553,7 +553,7 @@ case class StringTrim(
553553
trimStr: Option[Expression] = None)
554554
extends String2TrimExpression {
555555

556-
def this (trimStr: Expression, srcStr: Expression) = this(srcStr, Option(trimStr))
556+
def this(trimStr: Expression, srcStr: Expression) = this(srcStr, Option(trimStr))
557557

558558
def this(srcStr: Expression) = this(srcStr, None)
559559

@@ -570,9 +570,9 @@ case class StringTrim(
570570
null
571571
} else {
572572
if (trimStr.isDefined) {
573-
return srcString.trim(trimStr.get.eval(input).asInstanceOf[UTF8String])
573+
srcString.trim(trimStr.get.eval(input).asInstanceOf[UTF8String])
574574
} else {
575-
return srcString.trim()
575+
srcString.trim()
576576
}
577577
}
578578
}
@@ -582,7 +582,7 @@ case class StringTrim(
582582
val srcString = evals(0)
583583

584584
if (evals.length == 1) {
585-
ev.copy(evals.map(_.code).mkString("\n") + s"""
585+
ev.copy(evals.map(_.code).mkString + s"""
586586
boolean ${ev.isNull} = false;
587587
UTF8String ${ev.value} = null;
588588
if (${srcString.isNull}) {
@@ -599,8 +599,7 @@ case class StringTrim(
599599
} else {
600600
${ev.value} = ${srcString.value}.trim(${trimString.value});
601601
}"""
602-
ev.copy(evals.map(_.code).mkString("\n") +
603-
s"""
602+
ev.copy(evals.map(_.code).mkString + s"""
604603
boolean ${ev.isNull} = false;
605604
UTF8String ${ev.value} = null;
606605
if (${srcString.isNull}) {
@@ -623,7 +622,7 @@ object StringTrimLeft {
623622
* function will have one argument, which contains the source string.
624623
* If LEADING and trimStr keywords are not specified, it trims the characters from left end. The ltrim function will
625624
* have two arguments, the first argument contains trimStr, the second argument contains the source string.
626-
* trimStr: the function removes any characters from the left end of the source string which matches with the characters
625+
* trimStr: the function removes any character from the left end of the source string which matches with the characters
627626
* from trimStr, it stops at the first non-match character.
628627
* LEADING: removes any character from the left end of the source string that matches characters in the trim string.
629628
*/
@@ -668,9 +667,9 @@ case class StringTrimLeft(
668667
null
669668
} else {
670669
if (trimStr.isDefined) {
671-
return srcString.trimLeft(trimStr.get.eval(input).asInstanceOf[UTF8String])
670+
srcString.trimLeft(trimStr.get.eval(input).asInstanceOf[UTF8String])
672671
} else {
673-
return srcString.trimLeft()
672+
srcString.trimLeft()
674673
}
675674
}
676675
}
@@ -680,7 +679,7 @@ case class StringTrimLeft(
680679
val srcString = evals(0)
681680

682681
if (evals.length == 1) {
683-
ev.copy(evals.map(_.code).mkString("\n") + s"""
682+
ev.copy(evals.map(_.code).mkString + s"""
684683
boolean ${ev.isNull} = false;
685684
UTF8String ${ev.value} = null;
686685
if (${srcString.isNull}) {
@@ -697,8 +696,7 @@ case class StringTrimLeft(
697696
} else {
698697
${ev.value} = ${srcString.value}.trimLeft(${trimString.value});
699698
}"""
700-
ev.copy(evals.map(_.code).mkString("\n") +
701-
s"""
699+
ev.copy(evals.map(_.code).mkString + s"""
702700
boolean ${ev.isNull} = false;
703701
UTF8String ${ev.value} = null;
704702
if (${srcString.isNull}) {
@@ -721,14 +719,14 @@ object StringTrimRight {
721719
* rtrim function will have one argument, which contains the source string.
722720
* If TRAILING and trimStr keywords are specified, it trims the characters from right end. The rtrim function will
723721
* have two arguments, the first argument contains trimStr, the second argument contains the source string.
724-
* trimStr: the function removes any characters from the right end of source string which matches with the characters
722+
* trimStr: the function removes any character from the right end of source string which matches with the characters
725723
* from trimStr, it stops at the first non-match character.
726724
* TRAILING: removes any character from the right end of the source string that matches characters in the trim string.
727725
*/
728726
@ExpressionDescription(
729727
usage = """
730728
_FUNC_(str) - Removes the trailing space characters from `str`.
731-
_FUNC_(trimStr, str) - Removes the trailing string which contains the character from the trim string from the `str`
729+
_FUNC_(trimStr, str) - Removes the trailing string which contains the characters from the trim string from the `str`
732730
""",
733731
arguments = """
734732
Arguments:
@@ -766,9 +764,9 @@ case class StringTrimRight(
766764
null
767765
} else {
768766
if (trimStr.isDefined) {
769-
return srcString.trimRight(trimStr.get.eval(input).asInstanceOf[UTF8String])
767+
srcString.trimRight(trimStr.get.eval(input).asInstanceOf[UTF8String])
770768
} else {
771-
return srcString.trimRight()
769+
srcString.trimRight()
772770
}
773771
}
774772
}
@@ -778,7 +776,7 @@ case class StringTrimRight(
778776
val srcString = evals(0)
779777

780778
if (evals.length == 1) {
781-
ev.copy(evals.map(_.code).mkString("\n") + s"""
779+
ev.copy(evals.map(_.code).mkString + s"""
782780
boolean ${ev.isNull} = false;
783781
UTF8String ${ev.value} = null;
784782
if (${srcString.isNull}) {
@@ -795,8 +793,7 @@ case class StringTrimRight(
795793
} else {
796794
${ev.value} = ${srcString.value}.trimRight(${trimString.value});
797795
}"""
798-
ev.copy(evals.map(_.code).mkString("\n") +
799-
s"""
796+
ev.copy(evals.map(_.code).mkString + s"""
800797
boolean ${ev.isNull} = false;
801798
UTF8String ${ev.value} = null;
802799
if (${srcString.isNull}) {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,25 +1180,25 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
11801180
*/
11811181
override def visitFunctionCall(ctx: FunctionCallContext): Expression = withOrigin(ctx) {
11821182
def replaceFunctions(
1183-
funcID: FunctionIdentifier,
1184-
ctx: FunctionCallContext): FunctionIdentifier = {
1185-
val opt = ctx.trimOption
1186-
if (opt != null) {
1187-
if (ctx.qualifiedName.getText.toLowerCase != "trim") {
1188-
throw new ParseException(s"The specified function ${ctx.qualifiedName.getText} " +
1189-
s"doesn't support with option ${opt.getText}.", ctx)
1190-
}
1191-
opt.getType match {
1192-
case SqlBaseParser.BOTH => funcID
1193-
case SqlBaseParser.LEADING => funcID.copy(funcName = "ltrim")
1194-
case SqlBaseParser.TRAILING => funcID.copy(funcName = "rtrim")
1195-
case _ => throw new ParseException("Function trim doesn't support with " +
1196-
s"type ${opt.getType}. Please use BOTH, LEADING or Trailing as trim type", ctx)
1197-
}
1198-
} else {
1199-
funcID
1183+
funcID: FunctionIdentifier,
1184+
ctx: FunctionCallContext): FunctionIdentifier = {
1185+
val opt = ctx.trimOption
1186+
if (opt != null) {
1187+
if (ctx.qualifiedName.getText.toLowerCase(Locale.ROOT) != "trim") {
1188+
throw new ParseException(s"The specified function ${ctx.qualifiedName.getText} " +
1189+
s"doesn't support with option ${opt.getText}.", ctx)
12001190
}
1191+
opt.getType match {
1192+
case SqlBaseParser.BOTH => funcID
1193+
case SqlBaseParser.LEADING => funcID.copy(funcName = "ltrim")
1194+
case SqlBaseParser.TRAILING => funcID.copy(funcName = "rtrim")
1195+
case _ => throw new ParseException("Function trim doesn't support with " +
1196+
s"type ${opt.getType}. Please use BOTH, LEADING or Trailing as trim type", ctx)
1197+
}
1198+
} else {
1199+
funcID
12011200
}
1201+
}
12021202
// Create the function call.
12031203
val name = ctx.qualifiedName.getText
12041204
val isDistinct = Option(ctx.setQuantifier()).exists(_.DISTINCT != null)

0 commit comments

Comments
 (0)