Skip to content

Split the native core crate into focused per-concern crates (planner, expressions, readers, writers, storage, operators, shuffle, jni, proto, common) #5639

Description

@comphead

Summary

Split the native core crate (~31k LOC) into focused, single-concern crates with a strict one-way, acyclic dependency graph. This issue is scoped to the native Rust project structure and its dependencies only. It does not change behavior, wire format, or the JVM side.

The target crates are common, proto, jni-bridge, expr, storage, operators, readers, writers, shuffle, and planner, with the existing core crate (the cdylib) slimmed down to the JNI entry layer.

Problem

Today native/core fuses five concerns into one crate: the planner (a single 7141-line planner.rs), readers (parquet plus the scan operators), writers (parquet writer), storage (object store plus cloud credentials), and the JNI entry layer. Editing the Parquet reader recompiles the planner. There is no enforced dependency direction, so any part can reach into any other.

The other native crates (proto, spark-expr, shuffle, jni-bridge, common) are already separate and their graph is acyclic. The work is to break up core.

Proposed native structure

Each new crate is carved out of today's core. The core crate keeps its name and remains the cdylib, but shrinks to only the JNI entry points and the memory pools.

Crate Holds (from today's core) Depends on
comet-common errors, schema, arrow convert, query context, tracing, metrics leaf
comet-proto generated protobuf types (exists) leaf
comet-jni-bridge Rust↔JVM callbacks: task mem mgr, metric node, s3 cred, udf bridge, arrow stream (exists) common
comet-expr spark-expr plus execution/expressions/* common, jni-bridge
comet-storage parquet/objectstore/{s3,azure}, cloud/s3 common, jni-bridge
comet-operators expand, explode, projection, sample, rank_limit, copy common, expr
comet-readers parquet reader, csv/iceberg/shuffle scan, columnar_to_row, schema_adapter, cast_column common, expr, storage
comet-writers parquet_writer common, storage
comet-shuffle shuffle writer, partitioners, IPC (exists) common, jni-bridge, expr
comet-planner planner.rs, serde, spark_plan, registries proto, common, expr, operators, readers, writers, shuffle, storage
core (cdylib) lib.rs, jni_api, parquet JNI entry, memory_pools planner, jni-bridge, proto

Dependency graph

graph TB
  subgraph cdylib["cdylib"]
    core["core: jni entry + memory_pools"]
  end
  subgraph assembler["assembler"]
    planner["comet-planner"]
  end
  subgraph execution["execution"]
    direction LR
    readers["comet-readers"]
    writers["comet-writers"]
    operators["comet-operators"]
    shuffle["comet-shuffle"]
  end
  subgraph blocks["building blocks"]
    direction LR
    expr["comet-expr"]
    storage["comet-storage"]
  end
  subgraph ffi["ffi"]
    jnibridge["comet-jni-bridge"]
  end
  subgraph foundation["foundation"]
    direction LR
    common["comet-common"]
    proto["comet-proto"]
  end

  core --> planner
  core --> jnibridge
  core --> proto
  planner --> proto
  planner --> expr
  planner --> operators
  planner --> readers
  planner --> writers
  planner --> shuffle
  planner --> storage
  readers --> expr
  readers --> storage
  writers --> storage
  operators --> expr
  shuffle --> expr
  expr --> jnibridge
  storage --> jnibridge
  shuffle --> jnibridge
  jnibridge --> common
Loading

Topological order (proof it is acyclic): common, proto then jni-bridge then expr, storage then operators, readers, writers, shuffle then planner then core.

The one rule that keeps it acyclic: the proto-to-plan build logic lives in comet-planner. Content crates (expr, operators, readers, and the rest) never depend back on the planner.

Granularity choice

This proposal splits execution into readers + writers + operators + storage (shuffle is already separate). The finer split gives better incremental builds and clearer ownership. If that turns out to be too many crates, collapsing readers/writers/operators/storage into one comet-execution crate is a cheap merge later, since the dependency direction is unchanged.

Suggested split order

Each step ships with a green build and can merge independently. Do file relocations as pure git mv commits so blame follows.

  • Move ExecutionError (and SparkError, QueryContext) down into comet-common first. This fixes the current inversion where those types live in jni-bridge and spark-expr, and it removes the ~20 spurious edges that route through operators/mod.rs.
  • Extract comet-storage (objectstore plus cloud). It has no upward dependencies.
  • Fold execution/expressions/* into the expressions crate to form comet-expr.
  • Split comet-readers, comet-writers, and comet-operators out of execution/operators and parquet.
  • Extract comet-planner (planner.rs, serde, spark_plan, registries) as the assembler.
  • Reduce core to the JNI entry plus memory pools.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions