Rework FIXED_LIST #3057

manh9203 · 2024-03-14T16:36:25Z

This PR resolves the issue #2975

What is changed:

Rename FIXED_LIST to ARRAY
Fix ARRAY to use the same physical layout as VAR_LIST so it can support nested data types
Unify string casting functions of VAR_LIST and ARRAY
Unify casting functions between nested data types
Fix NpyReader to write to ListVector
Fix StorageDriver to read from VarListColumn

To-do list:

Implement implicit casting rules between VAR_LIST and ARRAY so we can use list functions for array
Support array native functions Feature Request: Vector Operations #3068
Optimize the physical layout of array by removing the entry information vector

tools/rust_api/src/ffi.rs

tools/java_api/src/main/java/com/kuzudb/KuzuDataTypeID.java

src/include/common/type_utils.h

src/include/common/types/types.h

src/common/type_utils.cpp

andyfengHKU · 2024-03-14T18:19:33Z

src/common/types/value/value.cpp

 void Value::copyFromVarList(ku_list_t& list, const LogicalType& childType) {
+    if (dataType->getLogicalTypeID() == LogicalTypeID::ARRAY) {


Let's discuss with @ray6080 to see if we should have this type of check everywhere in the code base. I tend to not have it. And maybe we should have ARRAY being a physical type as well.

Let's add a physical type ARRAY to avoid these checks.

I think we should do it in another PR. I still have array using the same physical layout as var-list in this PR. And I removed the size check since we didn't have these checks for the old fixed-list vector.

andyfengHKU · 2024-03-14T18:23:03Z

src/include/main/query_result.h

-    // Used by fixedList only.
-    uint64_t numValuesPerList;
+    // Used by array only.
+    uint64_t fixedNumValues;


I'm not sure should we remove it since this field is still needed in other parts of the code.

src/function/cast/cast_fixed_list.cpp

ray6080 · 2024-03-15T15:41:27Z

Don't forget to update docs or open an issue on docs repo.

codecov · 2024-03-17T23:03:33Z

Codecov Report

Attention: Patch coverage is 99.41860% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 92.94%. Comparing base (bd963c1) to head (35b9438).

Files	Patch %	Lines
src/common/arrow/arrow_row_batch.cpp	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3057      +/-   ##
==========================================
- Coverage   93.00%   92.94%   -0.06%     
==========================================
  Files        1155     1155              
  Lines       43286    42919     -367     
==========================================
- Hits        40259    39892     -367     
  Misses       3027     3027

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andyfengHKU · 2024-03-18T18:20:55Z

src/function/cast/cast_array.cpp

+
+    switch (resultType->getPhysicalType()) {
+    case PhysicalTypeID::VAR_LIST: {
+        if (inputType.getPhysicalType() == PhysicalTypeID::VAR_LIST) {


I would write as

if input == ARRAY && output == ARRAY check numElements return if input == LIST && output == ARRAY: check numElements return validate recursively

andyfengHKU · 2024-03-18T18:41:54Z

src/common/arrow/arrow_row_batch.cpp

-        numValuesInChild *
-        storage::StorageUtils::getDataTypeSize(LogicalType{typeInfo.childrenTypesInfo[0]->typeID}));
-    for (auto i = 0u; i < numValuesPerList; i++) {
+    if (typeInfo.childrenTypesInfo[0]->typeID != LogicalTypeID::VAR_LIST &&


Leave a comment say why we don't need to resize for VAR_LIST and ARRAY

manh9203 marked this pull request as draft March 14, 2024 16:37

manh9203 changed the title ~~Fixed-list rework~~ Rework FIXED_LIST Mar 14, 2024

andyfengHKU reviewed Mar 14, 2024

View reviewed changes

manh9203 force-pushed the fixed-list-rework branch from dff13f4 to bf78628 Compare March 16, 2024 03:18

manh9203 marked this pull request as ready for review March 16, 2024 03:21

manh9203 force-pushed the fixed-list-rework branch 2 times, most recently from 1c52155 to b90ff5d Compare March 18, 2024 05:04

manh9203 requested a review from andyfengHKU March 18, 2024 14:48

andyfengHKU approved these changes Mar 18, 2024

View reviewed changes

Rework Fixed-list

35b9438

manh9203 force-pushed the fixed-list-rework branch from b90ff5d to 35b9438 Compare March 18, 2024 19:27

manh9203 merged commit e7c6d73 into master Mar 18, 2024
17 checks passed

manh9203 deleted the fixed-list-rework branch March 18, 2024 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework FIXED_LIST #3057

Rework FIXED_LIST #3057

manh9203 commented Mar 14, 2024 •

edited

Loading

andyfengHKU Mar 14, 2024

ray6080 Mar 17, 2024

manh9203 Mar 18, 2024

andyfengHKU Mar 14, 2024

manh9203 Mar 18, 2024

ray6080 commented Mar 15, 2024

codecov bot commented Mar 17, 2024 •

edited

Loading

andyfengHKU Mar 18, 2024

andyfengHKU Mar 18, 2024

		void Value::copyFromVarList(ku_list_t& list, const LogicalType& childType) {
		if (dataType->getLogicalTypeID() == LogicalTypeID::ARRAY) {

Rework FIXED_LIST #3057

Rework FIXED_LIST #3057

Conversation

manh9203 commented Mar 14, 2024 • edited Loading

What is changed:

To-do list:

andyfengHKU Mar 14, 2024

Choose a reason for hiding this comment

ray6080 Mar 17, 2024

Choose a reason for hiding this comment

manh9203 Mar 18, 2024

Choose a reason for hiding this comment

andyfengHKU Mar 14, 2024

Choose a reason for hiding this comment

manh9203 Mar 18, 2024

Choose a reason for hiding this comment

ray6080 commented Mar 15, 2024

codecov bot commented Mar 17, 2024 • edited Loading

Codecov Report

andyfengHKU Mar 18, 2024

Choose a reason for hiding this comment

andyfengHKU Mar 18, 2024

Choose a reason for hiding this comment

manh9203 commented Mar 14, 2024 •

edited

Loading

codecov bot commented Mar 17, 2024 •

edited

Loading