apache / apache/datafusion

Improved performance for FixedSizeBinary grouping

Open
#24,267 1 comment 0 reactions 1 assignee Claimed by @dhruvxvaishnav View on GitHub
enhancement performance
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Is your feature request related to a problem or challenge?

- This is a follow on to https://github.com/apache/datafusion/pull/23646

@maxburke added native support for FixedSizeBinary grouping keys (for example uuids) and the initial support is faster than the previous implementation. While testing the PR, I found it can still be optimzied more, so I wanted to record the ideas in this ticket.

Here is a way to test:
```sql
COPY (
SELECT
arrow_cast(decode(md5((value % 5000000)::varchar), 'hex'), 'FixedSizeBinary(16)') AS uuid,
(value % 10) AS id
FROM generate_series(1, 200000000)
) TO 'uuids.parquet';
```

This looks like this

```shell
andrewlamb@Andrews-MacBook-Pro-3:~/Downloads$ du -s -h uuids.parquet
3.0G uuids.parquet
```

```sql
> select * from 'uuids.parquet' limit 10;
+----------------------------------+----+
| uuid | id |
+----------------------------------+----+
| ae3fcf8bfebbc45af3bf12f70d9d3acd | 5 |
| c94adce9d37cb5fde98966e278815c64 | 6 |
| 2bc049eddc1314ec3bee9509f935741b | 7 |
| 76b22168b7e87edca43468290be51218 | 8 |
| aa59f82ba5a34a3d3c25773d8a948e99 | 9 |
| 657aa03184c164a30af703a02e2a687b | 0 |
| 491f0cbcf7b88ac92d2adff2a0528d81 | 1 |
| 17da0cbc4ffef7f04b4e3a2283160fd7 | 2 |
| 81a1f57e643d79dfcee6cd4117d43c30 | 3 |
| f0b301dad7ddd5eb3ea3d854823cc798 | 4 |
+----------------------------------+----+
10 row(s) fetched.
Elapsed 0.021 seconds.
```

And then ran this query:

```sql
SELECT count(*) FROM (SELECT uuid, id FROM 'uuids.parquet' GROUP BY uuid, id);
```

### Describe the solution you'd like

Optimize it more

Here is one way to profile what is taking time using samply

```shell
samply record ./datafusion-cli-multi-group-by-fsb -c "SELECT count(*) FROM (SELECT uuid, id FROM 'uuids.parquet' GROUP BY uuid, id);"
```

Basically try to make this part faster:

Image

### Describe alternatives you've considered

Here is one possibility: https://github.com/apache/datafusion/pull/23646#discussion_r3752605833

> As a follow on, this can likely be optimized more -- for example, we could have a special case loop for when the inputs are known to have no nulls (likely a common case for things like UUIDs)
> We could also move to using get_unchecked to skip the bounds check and try to make this lookup loop even more performant

### Additional context

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.