feat: support fixed-size arrays in data readers#726
Conversation
# Conflicts: # src/storages/loader/loader_utils.cc
| | `escaping` | bool | `true` | Whether to enable escape character processing | | ||
|
|
||
| > **Array limitation:** `COPY FROM` CSV currently does not auto-detect or directly materialize fixed-size `ARRAY` columns from bracketed CSV fields such as `"[1,2,3]"`. Use Cypher literals/parameters for array properties, or a typed non-CSV ingestion path when available. | ||
| Since NeuG v0.1.3, `COPY FROM` supports loading `ARRAY` data from CSV, |
There was a problem hiding this comment.
- Do we supported nested Arrays?
- What is the expected format of string, to be parsed into a array? is there any best practices from other database systems?
There was a problem hiding this comment.
支持嵌套数组；string 数组的格式就是 '[1, 2, 3]' or '[[1, 2, 3]]' ，json/parquet 直接复用他们自己的数组类型
| static_cast<const arrow::Date32Array&>(array).Value(index)); | ||
| return Value::DATE(date); | ||
| } | ||
| case arrow::Type::DATE64: |
There was a problem hiding this comment.
[Medium] DATE64 construction may be incorrect
// DATE32: explicitly converts days since epoch
case arrow::Type::DATE32: {
Date date;
date.from_num_days(
static_cast<const arrow::Date32Array&>(array).Value(index));
return Value::DATE(date);
}
// DATE64: passes milliseconds directly to Date constructor
case arrow::Type::DATE64:
return Value::DATE(
Date(static_cast<const arrow::Date64Array&>(array).Value(index)));Date32Array::Value() returns days since epoch; Date64Array::Value() returns milliseconds since epoch. The DATE32 path correctly uses from_num_days(), but DATE64 passes raw milliseconds to the Date constructor. If the constructor expects days (not milliseconds), this produces a date far in the future.
Suggestion: Verify the Date constructor's parameter semantics. If it expects days, convert: date.from_num_days(value / 86400000). If it expects milliseconds, add a comment clarifying the difference from DATE32.
| for (int64_t i = 0; i < array->length(); ++i) { | ||
| if (array->IsNull(i)) { | ||
| THROW_NOT_SUPPORTED_EXCEPTION( | ||
| "Null Parquet ARRAY values are not supported"); |
There was a problem hiding this comment.
[Low] Null Parquet ARRAY values throw instead of being skipped
if (array->IsNull(i)) {
THROW_NOT_SUPPORTED_EXCEPTION(
"Null Parquet ARRAY values are not supported");
}CSV and JSON paths handle nulls via push_back_null(), but the Parquet path throws. If a Parquet file contains any null array values, the entire load fails. This inconsistency could surprise users.
Suggestion: Either use builder->push_back_null() for consistency with CSV/JSON, or document this limitation in the load_data / import_data docs.
|
|
||
| namespace neug { | ||
|
|
||
| static DataType arrow_type_to_neug_type(const arrow::DataType& type) { |
There was a problem hiding this comment.
[Low] arrow_type_to_neug_type and arrow_value_at are parallel switch-cases
Both functions (~50 lines each) manually map every Arrow type to a NeuG type or Value. Adding a new Arrow type requires updating both functions, and it's easy to forget one.
Suggestion: Consider adding a comment like // When adding a new Arrow type, update both arrow_type_to_neug_type and arrow_value_at. to help future maintainers.
There was a problem hiding this comment.
[Low]
arrow_type_to_neug_typeandarrow_value_atare parallel switch-casesBoth functions (~50 lines each) manually map every Arrow type to a NeuG type or Value. Adding a new Arrow type requires updating both functions, and it's easy to forget one.
Suggestion: Consider adding a comment like
// When adding a new Arrow type, update both arrow_type_to_neug_type and arrow_value_at.to help future maintainers.
all fixed in 486a112
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR adds end-to-end support for explicitly typed fixed-size arrays (T[N]) across CSV/JSON/Parquet readers, including nested arrays and length validation, and updates docs + CI to cover the workflow.
Changes:
- Add CSV parsing and property materialization support for fixed-size
ARRAYvalues (including nested arrays). - Add JSON reader support for fixed-size
ARRAYcolumns and unit tests. - Add Parquet (Arrow)
FIXED_SIZE_LISTsupport and expand end-to-end test coverage + docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/python_bind/tests/test_load_array.py | Adds comprehensive end-to-end tests for fixed-size array ingestion via CSV/JSON/Parquet. |
| tools/python_bind/tests/test_data_io_docs.py | Updates doc-driven tests to reflect new explicit-cast array workflow. |
| tests/utils/json_test.cc | Adds JSON reader unit test + helper for fixed-size array schema types. |
| src/utils/io/read/json/json_reader.cc | Implements JSON array parsing into fixed-size NeuG ARRAY values with length validation. |
| src/storages/loader/loader_utils.cc | Adds CSV array tokenization/parsing + wires array appender and property materialization. |
| src/storages/loader/csv_property_graph_loader.cc | Switches schema type plumbing to pass full DataType (enables arrays) into CSV reader meta. |
| include/neug/storages/loader/loader_utils.h | Updates reader-meta APIs to accept DataType instead of DataTypeId. |
| extension/parquet/src/arrow_column.cc | Adds Arrow fixed-size list conversion + nested array cell materialization; improves date64 handling. |
| doc/source/data_io/load_data.md | Documents explicit CAST(..., T[N]) workflow for arrays. |
| doc/source/data_io/import_data.md | Documents COPY FROM (LOAD FROM ... CAST(..., T[N])) workflow for arrays. |
| .github/workflows/neug-extension-test.yml | Adds Parquet array test invocation in extension workflow. |
| .github/workflows/build-and-test-common.yml | Adds array tests to main CI pytest suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export FLEX_DATA_DIR=${GITHUB_WORKSPACE}/example_dataset/comprehensive_graph | ||
| GLOG_v=10 ${GITHUB_WORKSPACE}/build/tools/utils/bulk_loader -g ../../example_dataset/comprehensive_graph/graph.yaml -l ../../example_dataset/comprehensive_graph/import.yaml -d /tmp/comprehensive_graph | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_load.py -k "parquet or httpfs" | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_load_array.py -k "parquet" | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_export.py -k "parquet or httpfs" |
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_query.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_tp_service.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_load.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_load_array.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_export.py |
| static std::shared_ptr<IContextColumn> convert_fixed_size_list_arrays( | ||
| const std::vector<std::shared_ptr<arrow::Array>>& arrays) { | ||
| const auto type = arrow_type_to_neug_type(*arrays.front()->type()); | ||
| auto builder = ColumnsUtils::create_builder(type); |
| NeuG supports reading `ARRAY` data from CSV, JSON, | ||
| and Parquet files. NeuG does not implicitly convert input values to `ARRAY`; | ||
| explicitly cast the column to a fixed-size `ARRAY` type in the `RETURN` | ||
| clause. |
| `COPY FROM` supports loading `ARRAY` data from CSV, | ||
| JSON, and Parquet files. NeuG does not implicitly convert input values to | ||
| `ARRAY`; use `LOAD FROM` and explicitly cast the column to a fixed-size | ||
| `ARRAY` type. |
| export FLEX_DATA_DIR=${GITHUB_WORKSPACE}/example_dataset/comprehensive_graph | ||
| GLOG_v=10 ${GITHUB_WORKSPACE}/build/tools/utils/bulk_loader -g ../../example_dataset/comprehensive_graph/graph.yaml -l ../../example_dataset/comprehensive_graph/import.yaml -d /tmp/comprehensive_graph | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_load.py -k "parquet or httpfs" | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_load_array.py -k "parquet" | ||
| NEUG_RUN_EXTENSION_TESTS=true python3 -m pytest -sv tests/test_export.py -k "parquet or httpfs" |
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_query.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_tp_service.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_load.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_load_array.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_export.py | ||
| python3 -m pytest --cov=neug --cov-report=term --exitfirst --cov-append --cov-config=.coveragerc -sv tests/test_copy_temp.py |
| static int64_t arrow_time_to_milliseconds(int64_t value, | ||
| arrow::TimeUnit::type unit) { | ||
| switch (unit) { | ||
| case arrow::TimeUnit::SECOND: | ||
| return value * Interval::MSECS_PER_SEC; | ||
| case arrow::TimeUnit::MILLI: | ||
| return value; | ||
| case arrow::TimeUnit::MICRO: | ||
| return value / Interval::MICROS_PER_MSEC; | ||
| case arrow::TimeUnit::NANO: | ||
| return value / (Interval::MICROS_PER_MSEC * 1000); | ||
| } | ||
| THROW_NOT_SUPPORTED_EXCEPTION("Unsupported Arrow time unit"); | ||
| } |
| static std::shared_ptr<IContextColumn> convert_fixed_size_list_arrays( | ||
| const std::vector<std::shared_ptr<arrow::Array>>& arrays) { | ||
| const auto type = arrow_type_to_neug_type(*arrays.front()->type()); | ||
| auto builder = ColumnsUtils::create_builder(type); | ||
| size_t size = 0; | ||
| for (const auto& array : arrays) { | ||
| size += array->length(); |
Summary
LOAD FROM, including nested arrays, quoted strings, temporal values, and length validationCOPY FROMresults into graph propertiesFIXED_SIZE_LISTcolumns in the Parquet reader, including nested arrays and scalar child typesCAST(..., T[N])workflowVariable-length
LISTtypes such asFLOAT[]remain unsupported by these readers. Users must explicitly specify a fixed-size array type such asFLOAT[3]; implicit LIST-to-ARRAY conversion is not performed.Testing
python3 -m pytest -q tools/python_bind/tests/test_load_array.py— 15 passedFixes #441
Fixes #442
Fixes #443