Repository navigation
Add support for TRIM BOTH/LEADING/TRAILING #935
Description
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
The more standard SQL datafusion supports, the easier it will be to integrate it into the rest of the analytics ecosystem
Describe the solution you'd like
I would like to be able to run the following queries without error:
cargo run --bin datafusion-cli-- works
SELECT TRIM(' bar ');
-- does not work (examples from mysql)
SELECT TRIM(LEADING 'x' FROM 'xxxbarxxx');
SELECT TRIM(BOTH 'x' FROM 'xxxbarxxx');
SELECT TRIM(TRAILING 'xyz' FROM 'barxxyz');Actual output:
> SELECT TRIM(' bar ');
+------------------------+
| trim(Utf8(" bar ")) |
+------------------------+
| bar |
+------------------------+
1 row in set. Query took 0.003 seconds.
> SELECT TRIM(LEADING 'x' FROM 'xxxbarxxx');
Plan("TRIM LEADING is not yet supported ")
> SELECT TRIM(BOTH 'x' FROM 'xxxbarxxx');
Plan("TRIM BOTH is not yet supported ")
> SELECT TRIM(TRAILING 'xyz' FROM 'barxxyz');
Plan("TRIM TRAILING is not yet supported ")Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
SQL (postgres and mysql) offer support for extended TRIM syntax, and with #934 DataFusion's SQL parser can parse this syntax.
Here are links to the references:
- https://dev.mysql.com/doc/refman/8.0/en/string-functions.html#function_trim
- https://www.postgresql.org/docs/current/functions-string.html
Here is a copy of the syntax from the mysql manual:
TRIM([{BOTH | LEADING | TRAILING} [remstr] FROM] str), TRIM([remstr FROM] str)
Returns the string str with all remstr prefixes or suffixes removed. If none of the specifiers BOTH, LEADING, or TRAILING is given, BOTH is assumed. remstr is optional and, if not specified, spaces are removed.
mysql> SELECT TRIM(' bar ');
-> 'bar'
mysql> SELECT TRIM(LEADING 'x' FROM 'xxxbarxxx');
-> 'barxxx'
mysql> SELECT TRIM(BOTH 'x' FROM 'xxxbarxxx');
-> 'bar'
mysql> SELECT TRIM(TRAILING 'xyz' FROM 'barxxyz');
-> 'barx'
Note that thanks to @seddonm1 DataFusion already has the ltrim and rtrim functions
So supporting this syntax:
> SELECT TRIM(TRAILING 'xyz' FROM 'barxxyz');
Plan("TRIM TRAILING is not yet supported ")Should end up creating the same plan as this (which works today):
> SELECT RTRIM('barxxyz', 'xyz');
+------------------------------------+
| rtrim(Utf8("barxxyz"),Utf8("xyz")) |
+------------------------------------+
| bar |
+------------------------------------+
1 row in set. Query took 0.003 seconds.
Activity
seddonm1 commentedon Aug 23, 2021
I believe this should be very easy now the parser supports it and just needs a tiny bit of plumbing and a few tests.
alamb commentedon Aug 23, 2021
Indeed -- a perfect good first issue
adsharma commentedon Aug 25, 2021
Does this look right?
adsharma@8c4bd15
After this code, I see a query plan that looks legit, but the output is not what I'd expect. Any hints on what's missing or how to debug it?
alamb commentedon Aug 25, 2021
@adsharma perhaps the arguments to ltrim are reversed? Maybe something like
ltrim(Utf8("xxxbarxxx"), Utf8("x"))would produce the expected answer?adsharma commentedon Aug 25, 2021
Thanks! That was one problem. Fixed now.
But there is a second significant problem:
MySQL would output "text".
The problem boils down to the interpretation: is it a set of characters or a string to be trimmed?
Perhaps address this in a new PR?
Add support for trim variants
alamb commentedon Aug 25, 2021
I think postgres
btrim(what DataFusion is modeled on) is supposed to be removing the substrings too (not a set of characters) 🤔 We will need to do some more research perhapsseddonm1 commentedon Aug 25, 2021
@alamb let me have a look
seddonm1 commentedon Aug 25, 2021
I had a look at the postgres docs again and this implementation meets their definition (https://www.postgresql.org/docs/current/functions-string.html#id-1.5.8.10.7.2.2.2.1.1.1) importantly:
removes the longest string containing only characters in characterswhich means characters is treated as an character array not a string literal
Both postgres and datafusion implement consistently:
Add support for trim variants
alamb commentedon Aug 26, 2021
I ran the same query in MYSQL and POSTGRES and they do produce different results (!!):
mysql
postgres
So as @seddonm1 points out DF is consistent with postgres which is the current target
Add support for trim variants (#947)