Skip to content

Commit

Permalink
Fixed TRIM/LTRIM/RTRIM/BTRIM on null 2nd arg
Browse files Browse the repository at this point in the history
Return `NULL` not only if the input string to trim is `NULL`, but also
if the `trimmingText` argument of the functions is `NULL`, thus
complying with PostgreSQL behaviour.

Fixes: #15527
  • Loading branch information
matriv committed Feb 8, 2024
1 parent 5e25b88 commit b73c945
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 3 deletions.
7 changes: 6 additions & 1 deletion docs/appendices/release-notes/5.6.2.rst
Expand Up @@ -45,9 +45,14 @@ See the :ref:`version_5.6.0` release notes for a full list of changes in the
Fixes
=====

- Fixed :ref:`trim <scalar-trim>`, :ref:`ltrim <scalar-ltrim>`,
:ref:`rtrim <scalar-rtrim>, and :ref:`btrim <scalar-btrim>` scalar functions
to return ``NULL`, instead of the original string, when the ``trimmingText``
argument is ``NULL``, complying with PostgreSQL behaviour for these functions.

- Fixed a regression introduced in 5.6.0 that caused
:ref:`concat_ws <scalar-concat-ws>` returning the wrong result when used on a
column with NULL values in the WHERE-clause combined with a NOT-predicate.
column with ``NULL`` values in the WHERE-clause combined with a NOT-predicate.
An example::

SELECT * FROM t1 WHERE NOT CONCAT_WS(true, column_with_null_value, false);
Expand Down
8 changes: 8 additions & 0 deletions docs/general/builtins/scalar-functions.rst
Expand Up @@ -480,6 +480,8 @@ Removes the longest string containing characters from ``str_arg_1`` (``' '`` by
default) from the start, end, or both ends (``BOTH`` is the default) of
``str_arg_2``.

If any of the two strings is ``NULL``, the result is ``NULL``.

Synopsis::

trim([ [ {LEADING | TRAILING | BOTH} ] [ str_arg_1 ] FROM ] str_arg_2)
Expand Down Expand Up @@ -523,6 +525,8 @@ Examples::
Removes set of characters which are matching ``trimmingText`` (``' '`` by
default) to the left of ``text``.

If any of the arguments is ``NULL``, the result is ``NULL``.

::

cr> select ltrim('xxxzzzabcba', 'xz') AS ltrim;
Expand All @@ -542,6 +546,8 @@ default) to the left of ``text``.
Removes set of characters which are matching ``trimmingText`` (``' '`` by
default) to the right of ``text``.

If any of the arguments is ``NULL``, the result is ``NULL``.

::

cr> select rtrim('abcbaxxxzzz', 'xz') AS rtrim;
Expand All @@ -562,6 +568,8 @@ A combination of :ref:`ltrim <scalar-ltrim>` and :ref:`rtrim <scalar-rtrim>`,
removing the longest string matching ``trimmingText`` from both the start and
end of ``text``.

If any of the arguments is ``NULL``, the result is ``NULL``.

::

cr> select btrim('XXHelloXX', 'XX') AS btrim;
Expand Down
Expand Up @@ -192,7 +192,7 @@ public Scalar<String, String> compile(List<Symbol> arguments, String currentUser
}

String charsToTrim = (String) ((Input<?>) charsToTrimSymbol).value();
if (charsToTrim.length() == 1) {
if (charsToTrim != null && charsToTrim.length() == 1) {
return new OneCharTrimFunction(signature, boundSignature, charsToTrim.charAt(0));
}
return this;
Expand All @@ -208,7 +208,7 @@ public String evaluate(TransactionContext txnCtx, NodeContext nodeCtx, Input<Str

String charsToTrimArg = args[1].value();
if (charsToTrimArg == null) {
return target;
return null;
}

TrimMode mode = TrimMode.of(args[2].value());
Expand Down Expand Up @@ -259,6 +259,8 @@ public String evaluate(TransactionContext txnCtx, NodeContext nodeCtx, Input<Str
String passedTrimmingText = args[1].value();
if (passedTrimmingText != null) {
return trimFunction.apply(target, passedTrimmingText);
} else {
return null;
}
}

Expand Down
Expand Up @@ -148,10 +148,21 @@ public void testEvaluateTrimWithoutCharsToTrim() {
@Test
public void testEvaluateNullInputOptimisedTrim() {
assertEvaluateNull("trim(name)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("trim('foo' FROM name)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("ltrim(name)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("rtrim(name)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("btrim(name)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("ltrim(name, null)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("rtrim(name, null)", Literal.of(DataTypes.STRING, null));
assertEvaluateNull("btrim(name, null)", Literal.of(DataTypes.STRING, null));
}

@Test
public void test_evaluate_null_second_arg_on_nonnull_input() {
assertEvaluateNull("trim(NULL FROM name)", Literal.of("foo"));
assertEvaluateNull("ltrim(name, null)", Literal.of("foo"));
assertEvaluateNull("rtrim(name, null)", Literal.of("foo"));
assertEvaluateNull("btrim(name, null)", Literal.of("foo"));
}

@Test
Expand Down

0 comments on commit b73c945

Please sign in to comment.