Internal error: The "character_length" function can only accept strings

[JayjeetAtGithub]

Describe the bug

On running the query below on the Clickbench multi file dataset,

SELECT "CounterID", AVG(length("URL")) AS l, COUNT(*) AS c FROM hits WHERE "URL" <> '' GROUP BY "CounterID" HAVING COUNT(*) > 100000 ORDER BY l DESC LIMIT 25;

we get this error,

Internal error: The "character_length" function can only accept strings.. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker

To Reproduce

Download the data using,

 ./benchmarks/bench.sh data clickbench_partitioned

A hits_multi directory with the parquet files will be created.

Execute the above queries,

datafusion-cli -c "CREATE EXTERNAL TABLE hits STORED AS PARQUET LOCATION 'hits_multi';" "{query}"

Expected behavior

The queries should run successfully without erroring.

Additional context

Datafusion 29.0.0

[parkma99]

It looks simple to fix, I will take it.

[alamb]

This looks similar to #7039 which @jonahgao fixed by adding a coercion from binary --> UTF8 for comparison. I think we could do something similar here.

[alamb]

Marking as a good first issue as there is a reproducer and I think the fix should be relatively straightforward

[alamb]

FWIW #7355 (review) will also make this a non internal error (though @parkma99 's real fix would be great)

[jhorstmann]

I'm wondering if an implicit cast is really the right solution here. Maybe a better solution would be to declare the schema / override the type in the CREATE TABLE statement.

Some other databases also have CHAR_LENGTH return the number of bytes on binary inputs: snowflake LENGTH function, Databricks CHARACTER_LENGTH

[jonahgao]

Declaring the datatype of URL as UTF8 is a quick fix.
Some DBMSs do this in Clickbench.

• clickhouse-parquet:
  https://github.com/ClickHouse/ClickBench/blob/9c6726e9ccb4cdb6f129a776ea5943235e465610/clickhouse-parquet/create.sql#L16

• duckdb-parquet has the binary_as_string option:
  https://github.com/ClickHouse/ClickBench/blob/9c6726e9ccb4cdb6f129a776ea5943235e465610/duckdb-parquet/create.sql#L6C1-L6C60

---

But I think it might also be necessary to make the LENGTH function support binaries.
PostgreSQL also supports it.

psql=> create table foo(c bytea);
CREATE TABLE
psql=> insert into foo values('bar');
INSERT 0 1
psql=> select length(c) from foo;
 length
--------
      3
(1 row)

[tustvold]

I personally find the notion of a function called character_length operating on binary data somewhat perverse, binary data by definition doesn't have a notion of a character.

Perhaps we could have a character length function that operates on strings, and a separate length kernel that returns the number of bytes - i.e. https://docs.rs/arrow-string/latest/arrow_string/length/fn.length.html ? A brief google search suggests this notion is normally called OCTET_LENGTH.

This would be consistent with postgres

I'm wondering if an implicit cast is really the right solution here.

I would second this, it feels like the clickbench setup may not be quite correct

PostgreSQL also supports it.

The docs would suggest that what is happening here is it is coercing the arguments to a string, I'll try to confirm this

[jonahgao]

The docs would suggest that what is happening here is it is coercing the arguments to a string, I'll try to confirm this

I have just checked the documentation and found that the length function in PostgresSQL has two different forms:

length(bytea) → integer
    Returns the number of bytes in the binary string.
    length('\x1234567890'::bytea) → 5

length(bytes bytea, encoding name) → integer
   Returns the number of characters in the binary string, assuming that it is text in the given encoding.
   length('jose'::bytea, 'UTF8') → 4

It seems that there will be no type coercion happening.

---

I think we might need three different types of length functions.

• character_length only operates on text strings.
• octet_length can be applied to both text strings and binaries, and it returns the length of the underlying byte array.
• length can be applied to both text strings and binaries.
  • For text strings, it has the same effect as character_length.
  • For binaries, it has the same effect as octet_length.

---

The following are the execution results of PostgreSQL for reference only.

postgres=#  create table text_table(c text);
CREATE TABLE
postgres=#  insert into text_table values('abc'), ('efg你好');
INSERT 0 2
postgres=# select length(c), octet_length(c), character_length(c) from text_table;
 length | octet_length | character_length
--------+--------------+------------------
      3 |            3 |                3
      5 |            9 |                5
(2 rows)

postgres=#
postgres=#
postgres=# create table binary_table(c bytea);
CREATE TABLE
postgres=#
postgres=# insert into binary_table values('abc'), ('\x00010203'::bytea);
INSERT 0 2
postgres=# select length(c), octet_length(c) from binary_table;
 length | octet_length
--------+--------------
      3 |            3
      4 |            4
(2 rows)

postgres=# select character_length(c) from binary_table;
ERROR:  function character_length(bytea) does not exist
LINE 1: select character_length(c) from binary_table;
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

[jonahgao]

I'm wondering if an implicit cast is really the right solution here. Maybe a better solution would be to declare the schema / override the type in the CREATE TABLE statement.

Yes, I think the simplest fix is to rewrite the query as

SELECT "CounterID", AVG(length(URL::TEXT)) AS l, COUNT(*) AS c FROM hits WHERE "URL" <> '' GROUP BY "CounterID" HAVING COUNT(*) > 100000 ORDER BY l DESC LIMIT 25;

We actually need character_length of text string here, so that the results should be consistent with others.
Calculating the length of a binary type may yield different results.

[alamb]

I personally find the notion of a function called character_length operating on binary data somewhat perverse, binary data by definition doesn't have a notion of a character.

Yes, this is why I think automatically coercing BinaryArray --> StringArray makes the most sense as during that process the binary data will be determined to be a string and then characters make sense