Skip to content

Add tests and clarify behavior for Parquet input (nodes/edges with multiple attributes) #17

Description

@shahar-biron

Summary

The bulk loader now supports reading from Parquet files via ParquetSource, normalizing them to the same row-based interface as CSV. However, this behavior is not currently covered by tests for labels and relation types, and some corner cases (e.g., multiple attributes per node/edge, NULL handling, complex Parquet types) are only implicitly documented.

What’s working today

  • ParquetSource uses pyarrow.parquet.read_table and exposes:
    • header as table.column_names.
    • iter_rows() yielding List[str] in header order, with "" for NULLs.
  • EntityFile (Label, RelationType) treat Parquet exactly like CSV:
    • Same header conversion logic (schema-less and schema-enforced).
    • Same pack_props and binary encoding for properties.
  • This means multiple attributes per label/type are already supported for Parquet by adding more columns, exactly like CSV.

Gaps

  1. No unit tests that exercise Parquet for:
    • Nodes with multiple property columns (schema-less and schema-enforced).
    • Relations with multiple property columns (schema-less and schema-enforced).
    • Basic NULL handling (None → "").
  2. Documentation does not call out that:
    • Parquet complex types (arrays/structs) will be str()-ed, which may not match the CSV conventions (e.g. "['Mon', 'Wed']" arrays expected by the loader).

Proposed work

  1. Add test coverage in test/:
    • Use pyarrow in tests to generate small Parquet files for:
      • Label with ID + multiple property columns.
      • Relation type with start/end IDs + multiple property columns.
    • Assert that:
      • Label / RelationType created from .parquet files report the expected column_names, prop_count, and entities_count.
      • Property values are correctly serialized (at least type-wise and count-wise).
  2. Documentation tweaks:
    • In the "Loading from Parquet files" section, add a short note that:
      • All values are converted via str().
      • Complex Parquet types (lists/maps/structs) may not map 1:1 to CSV-based array conventions, and may need explicit modeling or preprocessing.

Benefits

  • Guarantees functional parity between CSV and Parquet inputs for common scenarios.
  • Makes Parquet support safer to rely on in production, and makes expectations clearer for advanced types.

I have opened a PR implementing tests + docs for review.



This repo is using Opire - what does it mean? 👇
💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #17 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @shahar-biron (replace 20 with the amount, and @shahar-biron with the user to tip).

📖 If you want to learn more, check out our documentation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions