suneet-s commented on a change in pull request #10006:
URL: https://github.com/apache/druid/pull/10006#discussion_r437080564
##########
File path:
core/src/test/java/org/apache/druid/java/util/common/StringUtilsTest.java
##########
@@ -211,39 +211,51 @@ public void testRepeat()
@Test
public void testLpad()
{
- String s1 = StringUtils.lpad("abc", 7, "de");
- Assert.assertEquals(s1, "dedeabc");
+ String lpad = StringUtils.lpad("abc", 7, "de");
+ Assert.assertEquals("dedeabc", lpad);
- String s2 = StringUtils.lpad("abc", 6, "de");
- Assert.assertEquals(s2, "dedabc");
+ lpad = StringUtils.lpad("abc", 6, "de");
+ Assert.assertEquals("dedabc", lpad);
- String s3 = StringUtils.lpad("abc", 2, "de");
- Assert.assertEquals(s3, "ab");
+ lpad = StringUtils.lpad("abc", 2, "de");
+ Assert.assertEquals("ab", lpad);
- String s4 = StringUtils.lpad("abc", 0, "de");
- Assert.assertEquals(s4, "");
+ lpad = StringUtils.lpad("abc", 0, "de");
+ Assert.assertEquals("", lpad);
- String s5 = StringUtils.lpad("abc", -1, "de");
- Assert.assertEquals(s5, null);
+ lpad = StringUtils.lpad("abc", -1, "de");
+ Assert.assertNull(lpad);
+
+ lpad = StringUtils.lpad("abc", 10, "");
+ Assert.assertNull(lpad);
+
+ lpad = StringUtils.lpad("abc", 1, "");
+ Assert.assertNull(lpad);
}
@Test
public void testRpad()
{
- String s1 = StringUtils.rpad("abc", 7, "de");
- Assert.assertEquals(s1, "abcdede");
+ String rpad = StringUtils.rpad("abc", 7, "de");
+ Assert.assertEquals("abcdede", rpad);
+
+ rpad = StringUtils.rpad("abc", 6, "de");
+ Assert.assertEquals("abcded", rpad);
+
+ rpad = StringUtils.rpad("abc", 2, "de");
+ Assert.assertEquals("ab", rpad);
- String s2 = StringUtils.rpad("abc", 6, "de");
- Assert.assertEquals(s2, "abcded");
+ rpad = StringUtils.rpad("abc", 0, "de");
+ Assert.assertEquals("", rpad);
- String s3 = StringUtils.rpad("abc", 2, "de");
- Assert.assertEquals(s3, "ab");
+ rpad = StringUtils.rpad("abc", -1, "de");
+ Assert.assertNull(rpad);
- String s4 = StringUtils.rpad("abc", 0, "de");
- Assert.assertEquals(s4, "");
+ rpad = StringUtils.lpad("abc", 10, "");
Review comment:
🤦 bad copy paste
##########
File path: core/src/main/java/org/apache/druid/java/util/common/StringUtils.java
##########
@@ -481,9 +482,10 @@ public static String repeat(String s, int count)
*
* @return the string left-padded with pad to a length of len
*/
- public static String lpad(String base, Integer len, String pad)
+ @Nullable
+ public static String lpad(@Nonnull String base, int len, @Nonnull String pad)
{
- if (len < 0) {
+ if (len < 0 || pad.isEmpty()) {
Review comment:
Yeah I thought I made the change because FunctionTest was passing. Fixed
now.
I debugged it, and saw that FunctionTest passes because it treats the empty
string as null ( probably the default null handling mode in the Expr system),
so the rpad function thinks the `pad` that is passed in is null, not empty.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
