Skip to content

Commit

Permalink
Fix creation of DuckDB tables with arrays of boolean, tinyint and sma…
Browse files Browse the repository at this point in the history
…llint (#8408)

Summary:
Creating DuckDB table with a column of type array(boolean), array(tinyint) or
array(smallint) having a mix of null and non-null array elements used to fail with

```
INTERNAL Error: Assertion triggered in file "/home/runner/work/velox/velox/_build/debug/_deps/duckdb-src/src/common/types/value.cpp" on line 702: values[i].type() == values[0].type()
```

The problem is that `duckValueAt<TypeKind>` template in QueryAssertions.cpp used
to convert non-null values by calling duckdb::Value(v) API, which is not defined
for bool, int8_t and int16_t. Hence, we ended up calling a version of that API
that takes int32_t and returns a value of type INTEGER. However, code for
converting null values used duckdb::Value(type) API which takes type directly
and therefore creates values of the "right" type. When array value had a mix of
null and non-null values we ended up passing a list of elements of different
types to DuckDB and hit an assertion.

A fix implemented here is to provide explicit overrides for boolean, tinyint and smallint.

This issue happens often in join fuzzer runs.

Fixes #7943

Pull Request resolved: #8408

Reviewed By: amitkdutta

Differential Revision: D52826343

Pulled By: mbasmanova

fbshipit-source-id: 41d78f7cde95ed3f8e4b3fe3db980f5b60b52f47
  • Loading branch information
mbasmanova authored and facebook-github-bot committed Jan 17, 2024
1 parent e2af9d7 commit 82bcd39
Showing 1 changed file with 24 additions and 0 deletions.
24 changes: 24 additions & 0 deletions velox/exec/tests/utils/QueryAssertions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,30 @@ ::duckdb::Value duckValueAt(const VectorPtr& vector, vector_size_t index) {
return ::duckdb::Value(vector->as<SimpleVector<T>>()->valueAt(index));
}

template <>
::duckdb::Value duckValueAt<TypeKind::TINYINT>(
const VectorPtr& vector,
vector_size_t index) {
return ::duckdb::Value::TINYINT(
vector->as<SimpleVector<int8_t>>()->valueAt(index));
}

template <>
::duckdb::Value duckValueAt<TypeKind::SMALLINT>(
const VectorPtr& vector,
vector_size_t index) {
return ::duckdb::Value::SMALLINT(
vector->as<SimpleVector<int16_t>>()->valueAt(index));
}

template <>
::duckdb::Value duckValueAt<TypeKind::BOOLEAN>(
const VectorPtr& vector,
vector_size_t index) {
return ::duckdb::Value::BOOLEAN(
vector->as<SimpleVector<bool>>()->valueAt(index));
}

template <>
::duckdb::Value duckValueAt<TypeKind::VARCHAR>(
const VectorPtr& vector,
Expand Down

0 comments on commit 82bcd39

Please sign in to comment.