You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/26 21:15:09 UTC

[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6759: Support fixed_size_list for make_array

alamb commented on code in PR #6759:
URL: https://github.com/apache/arrow-datafusion/pull/6759#discussion_r1242772947


##########
Cargo.toml:
##########
@@ -53,6 +53,14 @@ arrow-array = { version = "42.0.0", default-features = false, features = ["chron
 parquet = { version = "42.0.0", features = ["arrow", "async", "object_store"] }
 sqlparser = { version = "0.35", features = ["visitor"] }
 
+[patch.crates-io]

Review Comment:
   I think we will have to wait for this code to be released to crates.io (in about 10 days) before merging this PR



##########
datafusion/core/tests/data/fixed_size_list_array.parquet:
##########


Review Comment:
   Did you look at the data sets available in https://github.com/apache/parquet-testing (which is a submodule of this repo)
   
   e.g. https://github.com/apache/parquet-testing/tree/a11fc8f148f8a7a89d9281cc0da3eb9d56095fbf/data



##########
datafusion/core/tests/sqllogictests/test_files/array.slt:
##########
@@ -357,3 +357,33 @@ query ?
 select make_array(x, y) from foo2;
 ----
 [1.0, 1]
+
+statement ok
+CREATE EXTERNAL TABLE fixed_size_list_array STORED AS PARQUET LOCATION 'tests/data/fixed_size_list_array.parquet';
+
+query ?
+select * from fixed_size_list_array;

Review Comment:
   Could you also add a query that is like `arrow_typeof(f0)` to show in the test what the type of the column is?



##########
datafusion/sql/src/expr/arrow_cast.rs:
##########
@@ -486,6 +497,8 @@ impl<'a> Tokenizer<'a> {
             "Date32" => Token::SimpleType(DataType::Date32),
             "Date64" => Token::SimpleType(DataType::Date64),
 
+            "List" => Token::List,

Review Comment:
   👍 



##########
datafusion/common/src/scalar.rs:
##########
@@ -3646,29 +3675,9 @@ impl fmt::Display for ScalarValue {
             ScalarValue::TimestampNanosecond(e, _) => format_option!(f, e)?,
             ScalarValue::Utf8(e) => format_option!(f, e)?,
             ScalarValue::LargeUtf8(e) => format_option!(f, e)?,
-            ScalarValue::Binary(e) => match e {

Review Comment:
   ❤️ 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org