Fix creation of DuckDB tables with arrays of boolean, tinyint and smallint #8408
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
the
CLA Signed
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
xiaoxmeng
approved these changes
Jan 17, 2024
mbasmanova
force-pushed
the
fix-join-fuzzer
branch
from
72f9bf2 to
41afcd3
Compare
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
kgpai
approved these changes
Jan 17, 2024
| @@ -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>( | |||
There was a problem hiding this comment.
Nit: should we add a test in QueryAssertionsTest
|
@mbasmanova merged this pull request in 82bcd39. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
The problem is that
duckValueAt<TypeKind>template in QueryAssertions.cpp usedto 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