refactor: clarify module boundaries for property types and export IO

[luoxiaojian]

No description provided.

[Copilot]

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.

Refactors Neug’s type/property module boundaries and shifts IO/export pipelines from protobuf QueryResponse/execution::Context-centric APIs toward DataChunk-based read/write and a dedicated export materialization step.

Changes:

• Repoints broad swaths of code from neug/utils/property/types.h to neug/common/types/property_types.h, and from neug/utils/property/* to neug/storages/property/*.
• Updates CSV/JSON/Parquet readers to return std::vector<DataChunk> and adds execution::Context::append_chunks(...) to consume them.
• Refactors export writers (CSV/JSON/Parquet) to write from DataChunk + source_types, introducing materialize_result_for_export(...) and adding new export-focused tests/utilities.

Reviewed changes

Copilot reviewed 132 out of 133 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tools/python_bind/tests/test_load_array.py	Skip test when PyArrow can’t write null fixed-size lists
tools/python_bind/src/py_query_result.cc	Switch property types include to common/types/property_types.h
tools/python_bind/src/py_query_request.cc	Switch property types include to common/types/property_types.h
tools/nodejs_bind/src/node_query_result.cc	Switch property types include to common/types/property_types.h
tools/nodejs_bind/src/node_query_request.cc	Switch property types include to common/types/property_types.h
tests/utils/test_types.cc	Update property storage includes to storages/property/*
tests/utils/test_table.cc	Update property storage includes to storages/property/*
tests/utils/test_reader.cc	Migrate reader tests to ctx.append_chunks(reader->read(...))
tests/utils/json_test.cc	Migrate JSON reader tests to append_chunks API
tests/utils/export_result_test.cc	New tests for export materialization + CSV/JSON output
tests/utils/CMakeLists.txt	Add export_result_test.cc to utils test target
tests/unittest/utils.h	Move property/id_indexer includes to new module locations
tests/unittest/test_indexer.cc	Switch id_indexer include to storages/id_indexer.h
tests/unittest/test_column.cc	Switch property column includes to storages/property/*
tests/unittest/schema_test.cc	Switch property types include to common/types/property_types.h
tests/unittest/logical_delete_test.cc	Switch property types include to common/types/property_types.h
tests/storage/test_vertex_table.cc	Switch property types include to common/types/property_types.h
tests/storage/test_checkpoint.cc	Switch property column include to storages/property/column.h
tests/storage/alter_property_test.cc	Switch property types include to common/types/property_types.h
src/utils/yaml_utils.cc	Switch property types include to common/types/property_types.h
src/utils/property/types.cc	Switch implementation include to common/types/property_types.h
src/utils/property/table.cc	Repoint table include to storages/property/table.h
src/utils/property/default_value.cc	Repoint default value include to common/types/default_value.h
src/utils/property/column.cc	Repoint property/id_indexer/types includes to new locations
src/utils/property/array_column.cc	Repoint array column/types include to new locations
src/utils/pb_utils.cc	Repoint default_value/property_types includes to common/types/*
src/utils/io/write/writer.cc	Rework CSV writer to format DataChunk cells; remove proto formatting
src/utils/io/read/json/json_reader.cc	Change JSON reader to return std::vector<DataChunk>
src/utils/io/read/csv/csv_reader.cc	Change CSV reader to return std::vector<DataChunk>
src/utils/bolt_utils.cc	Switch property types include to common/types/property_types.h
src/transaction/update_transaction.cc	Repoint property/id_indexer/types includes to new locations
src/transaction/insert_transaction.cc	Switch property types include to common/types/property_types.h
src/storages/property/CMakeLists.txt	New object library for property storage sources
src/storages/loader/loader_utils.cc	Repoint property column include to storages/property/column.h
src/storages/graph/schema.cc	Repoint property types/id_indexer includes to new locations
src/storages/graph/property_graph.cc	Repoint types/indexers/property column includes to new locations
src/storages/graph/edge_table.cc	Switch property types include to common/types/property_types.h
src/storages/csr/mutable_csr.cc	Switch property types include to common/types/property_types.h
src/storages/csr/immutable_csr.cc	Switch property types include to common/types/property_types.h
src/storages/csr/csr_view_utils.cc	Switch property types include to common/types/property_types.h
src/storages/CMakeLists.txt	Add storages/property subdir + object files to storages build
src/main/session.cc	Switch property types include to common/types/property_types.h
src/execution/utils/pb_parse_utils.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/vertex.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/tc_fuse.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/select.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/scan.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/group_by.cc	Switch property types include to common/types/property_types.h
src/execution/execute/ops/retrieve/edge.cc	Switch property types include to common/types/property_types.h
src/execution/common/operators/retrieve/sink.cc	Add export materialization helpers + materialize_result_for_export
src/execution/common/operators/retrieve/path_expand_impl.cc	Switch property types include to common/types/property_types.h
src/execution/common/operators/retrieve/join.cc	Switch property types include to common/types/property_types.h
src/execution/common/context.cc	Add Context::append_chunks and Context::column_types
src/compiler/function/json_export_function.cc	Rewrite JSON export to write from DataChunk + source_types
src/compiler/function/csv_export_function.cpp	CSV export now materializes export result then writes chunk/types
src/compiler/binder/bind/copy/bind_copy_from.cpp	Repoint default_value include to common/types/default_value.h
src/compiler/binder/bind/bind_ddl.cpp	Repoint default_value include to common/types/default_value.h
src/common/types/graph_types.cc	Switch property types include to common/types/property_types.h
include/neug/utils/string_utils.h	Switch property types include to common/types/property_types.h
include/neug/utils/service_utils.h	Switch property types include to common/types/property_types.h
include/neug/utils/serialization/out_archive.h	Switch property types include to common/types/property_types.h
include/neug/utils/serialization/in_archive.h	Switch property types include to common/types/property_types.h
include/neug/utils/property/types.h	Adjust Interval::normalize() declaration
include/neug/utils/property/table.h	Repoint property column/types includes to new locations
include/neug/utils/property/property_definition.h	Switch property types include to common/types/property_types.h
include/neug/utils/property/column.h	Switch property types include to common/types/property_types.h
include/neug/utils/property/array_column.h	Repoint column include to storages/property/column.h
include/neug/utils/pb_utils.h	Switch property types include to common/types/property_types.h
include/neug/utils/io/write/writer.h	ExportWriter now writes DataChunk + source_types; new CSV buffer
include/neug/utils/io/read/json/json_reader.h	JSON reader now returns std::vector<DataChunk>
include/neug/utils/io/read/json/json_read_config.h	Switch property types include to common/types/property_types.h
include/neug/utils/io/read/csv/csv_reader.h	CSV reader now returns std::vector<DataChunk>
include/neug/utils/io/read/csv/csv_read_config.h	Switch property types include to common/types/property_types.h
include/neug/utils/io/read/common/type_converter.h	Switch property types include to common/types/property_types.h
include/neug/utils/indexers.h	Repoint includes to storages/id_indexer.h + common/types/property_types.h
include/neug/utils/id_indexer.h	Repoint property includes to storages/property/* + common/types/property_types.h
include/neug/transaction/wal/wal_builder.h	Switch property types include to common/types/property_types.h
include/neug/transaction/wal/wal.h	Switch property types include to common/types/property_types.h
include/neug/transaction/update_transaction.h	Repoint property includes to storages/property/table.h + property_types
include/neug/transaction/transaction_utils.h	Switch property types include to common/types/property_types.h
include/neug/transaction/read_transaction.h	Repoint property includes to storages/property/* + property_types
include/neug/transaction/insert_transaction.h	Switch property types include to common/types/property_types.h
include/neug/transaction/compact_transaction.h	Switch property types include to common/types/property_types.h
include/neug/storages/module/type_name.h	Switch property types include + doc reference update
include/neug/storages/loader/loading_config.h	Switch property types include to common/types/property_types.h
include/neug/storages/loader/i_fragment_loader.h	Repoint indexers/types includes to new locations
include/neug/storages/loader/abstract_property_graph_loader.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/vertex_timestamp.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/vertex_table.h	Repoint indexers/table include to storages/*
include/neug/storages/graph/schema.h	Repoint default_value/property_types includes to common/types/*
include/neug/storages/graph/property_graph_cow_state.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/property_graph.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/operation_params.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/graph_view.h	Repoint property column include to storages/property/column.h
include/neug/storages/graph/graph_interface.h	Switch property types include to common/types/property_types.h
include/neug/storages/graph/edge_table.h	Repoint indexers/table include to storages/* + property_types
include/neug/storages/graph/dirty_tracker.h	Switch property types include to common/types/property_types.h
include/neug/storages/csr/nbr.h	Repoint property includes to storages/property/* + property_types
include/neug/storages/csr/mutable_csr.h	Switch property types include to common/types/property_types.h
include/neug/storages/csr/immutable_csr.h	Switch property types include to common/types/property_types.h
include/neug/storages/csr/csr_view.h	Repoint property column include to storages/property/column.h
include/neug/storages/csr/csr_base.h	Switch property types include to common/types/property_types.h
include/neug/storages/container/i_container.h	Switch property types include to common/types/property_types.h
include/neug/storages/allocators.h	Switch property types include to common/types/property_types.h
include/neug/storages/README.md	Fix doc links after module moves
include/neug/main/neug_db.h	Switch property types include to common/types/property_types.h
include/neug/execution/expression/special_predicates.h	Switch property types include to common/types/property_types.h
include/neug/execution/execute/ops/retrieve/scan_utils.h	Switch property types include to common/types/property_types.h
include/neug/execution/execute/ops/ddl/ddl_utils.h	Switch property types include to common/types/property_types.h
include/neug/execution/execute/ops/batch/batch_update_utils.h	Switch property types include to common/types/property_types.h
include/neug/execution/common/context.h	Declare append_chunks + column_types
include/neug/compiler/function/import/json_read_function.h	Consume new reader API via append_chunks
include/neug/compiler/function/import/csv_read_function.h	Consume new reader API via append_chunks
include/neug/compiler/function/export/json_export_function.h	Update writer API usage (DataChunk-based)
include/neug/common/types/i_context_column.h	Switch property types include to common/types/property_types.h
include/neug/common/types/graph_types.h	Switch property types include to common/types/property_types.h
include/neug/common/export/export_result.h	New exported ExportResult + materialize_result_for_export declaration
include/neug/common/columns/value_columns.h	Switch property types include to common/types/property_types.h
include/neug/common/columns/edge_columns.h	Remove old property column include; use property_types
extension/pattern_matching/include/fastest_lib/src/DataStructure/graph.h	Switch property types include to common/types/property_types.h
extension/parquet/tests/parquet_test.cc	Use append_chunks + new Parquet export test utils
extension/parquet/tests/parquet_export_test_utils.h	New utils: convert proto QueryResponse to DataChunk for tests
extension/parquet/src/parquet_export_function.cc	Rewrite Parquet export to write from DataChunk materialized export
extension/parquet/src/arrow_reader.cc	Reader now returns std::vector<DataChunk>; schema reconciliation changes
extension/parquet/src/arrow_options.cc	Use existing dataset_schema when projecting columns
extension/parquet/src/arrow_column.cc	Normalize interval values when converting Arrow durations
extension/parquet/include/parquet_read_function.h	Consume new reader API via append_chunks
extension/parquet/include/parquet_export_function.h	Parquet writer now writes DataChunk
extension/parquet/include/parquet/arrow_reader.h	Reader base/ArrowReader now returns std::vector<DataChunk>
extension/gds/include/utils/parallel_utils.h	Switch property types include to common/types/property_types.h
extension/gds/include/impl/louvain_impl.h	Switch property types include to common/types/property_types.h
extension/gds/include/impl/leiden_impl.h	Switch property types include to common/types/property_types.h

Comments suppressed due to low confidence (4)

src/execution/common/operators/retrieve/sink.cc:1

• materialize_column_for_export is defined inside an anonymous namespace nested under neug::execution earlier in this file, so it is not visible as execution::materialize_column_for_export (qualified lookup won’t find symbols in the unnamed namespace). This should fail to compile; fix by moving materialize_column_for_export into the neug::execution namespace (non-anonymous) or by calling it unqualified from a scope where it’s visible (or providing a properly-declared helper in neug::execution with internal linkage).
  src/execution/common/context.cc:1
• The docstring in include/neug/execution/common/context.h says this returns “Column logical types in tag_ids order”, but this implementation skips any tag_id not found in any chunk (it never pushes a placeholder), so the returned vector can be shorter than tag_ids and no longer index-aligned. Either (mandatory) return a vector with exactly tag_ids.size() entries (e.g., push a sentinel/default type when missing) or (optional) adjust the comment and rename the function to reflect that missing tags are omitted.
  tests/utils/export_result_test.cc:1
• Using a hard-coded shared directory under /tmp can lead to test flakiness when tests run concurrently (parallel gtest, multiple CI workers on the same host, or repeated runs overlapping), since SetUp()/TearDown() aggressively remove_all() the same path. Use a unique per-test directory (e.g., include PID + test name, or use a temp directory helper) to avoid cross-test interference.
  include/neug/utils/io/write/writer.h:1
• writeWithEscapes does not modify the escape set, so the parameter should be const char* (or a fixed-size container/span) instead of char*. This prevents accidental mutation and allows passing string literals without casts.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 136 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

src/execution/common/operators/retrieve/sink.cc:1

• execution::materialize_column_for_export(...) is not declared in the neug::execution namespace in this diff: it’s defined inside an unnamed namespace earlier in this file (internal linkage). This will not compile. Move materialize_column_for_export (and any helpers it needs) out of the unnamed namespace into namespace neug::execution (or call it unqualified from a scope where it’s visible), and ensure there’s a matching declaration if it needs to be referenced across translation units.
  tests/utils/export_result_test.cc:1
• Using a fixed hardcoded directory under /tmp can make tests flaky when run in parallel (collisions between test processes) and non-portable on non-POSIX platforms. Prefer deriving a unique per-test directory from std::filesystem::temp_directory_path() plus pid/random suffix (or use a test framework temp-dir utility) and keep the existing cleanup logic.
  src/execution/common/operators/retrieve/sink.cc:1
• vcol (and similar casts for edge/path) is dereferenced without checking the dynamic_pointer_cast result. If a column reports elem_type() == kVertex but isn’t an IVertexColumn (e.g. representation changes or a ValueColumn<vertex_t> is used), this will null-deref and crash. Add a concrete check and throw a descriptive exception when the cast fails.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 136 changed files in this pull request and generated 1 comment.