Skip to content

refactor: stream ContextChunk between operators - #1039

Closed
liulx20 wants to merge 20 commits into
alibaba:mainfrom
liulx20:codex/stream-data-chunk
Closed

liulx20 wants to merge 20 commits into
alibaba:mainfrom
liulx20:codex/stream-data-chunk

Conversation

@liulx20

@liulx20 liulx20 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

What do these changes do?

Operators currently exchange materialized Context values, which prevents downstream consumers from pulling individual batches through a pipeline. This change makes operator inputs and outputs Stream<ContextChunk> and connects nested pipelines through the same pull interface.

  • Return Stream directly from Eval and ExecuteStream. Defer fallible execution initialization until Next, preserve upstream DDL-before-COPY ordering, and report initialization and read errors through the same terminal stream error path.
  • Process row-local operators directly on chunks, retaining their columns and anonymous head without per-operator Context conversions. LIMIT tracks rows across batches and stops pulling once satisfied.
  • Keep explicit buffering for global operations and mutation boundaries that require stable input; materialize the final public query result at the query boundary.
  • Require a supplier factory for every registered reader. CSV, JSON, JSONL, and Parquet share this source path; remove the legacy materialized reader callback and fallback. Parquet consumes RecordBatchReader without a CountRows or ToTable pass.
  • Remove reader read(..., Context&) methods, their full-collection helpers, unused ReadLocalState, and the obsolete batch_read option. Reader tests pull suppliers directly, including interleaved independent Parquet suppliers.
  • Add coverage for stream termination, batch boundaries, early LIMIT, COPY FROM rollback, and reader lifetime.

This changes the operator and reader registration interfaces. It does not make every underlying algorithm incremental: global operators still buffer, CSV retains its row-count prepass, and JSON retains its DOM parser. Existing non-reader extension callback boundaries still use Context.

Validation

  • Release build: execution_test, utils_test, and neug_py_bind succeeded.
  • C++: 122 execution tests, 296 utility tests, and 178 compiler tests passed.
  • Python: 233 passed, 25 skipped, 10 HTTP/async cases deselected (HTTP server disabled).
  • Parquet extension sources and tests compiled and linked separately with PyArrow 18.0.0 dependencies: 37/38 passed, including the new supplier lifetime and individual-batch test. TestIntegration_ReadZstdWithSmallBufferedStream failed with Cannot shrink read buffer if buffered data remains; the prebuilt dependency lacks the repository's GH-48311 Arrow patch. This case has not been verified with patched Arrow.

Related issue number

Related to #1002; no issue is automatically closed.

@liulx20 liulx20 changed the title refactor(execution): stream ContextChunk between operators refactor: stream ContextChunk between operators Sep 8, 2026
liulx20 added a commit that referenced this pull request Sep 9, 2026
## What do these changes do?

A clean parallel extension build can compile `neug_parquet_carquet_impl`
before protobuf generation finishes. Its input/output stream headers
include `neug/utils/result.h`, which requires the generated
`neug/generated/proto/plan/error.pb.h`, causing a missing-header
compilation failure.

Add an explicit dependency on `neug_proto` so generation completes
before the Carquet adapters compile. This changes only one line in the
Parquet CMake configuration.

### Validation

A focused clean CMake build using the adapter target definition and the
actual adapter sources reproduced the missing-header failure without the
dependency. With the dependency, it generated `error.pb.h` and compiled
both adapter sources successfully with `-j4`. Full extension CI remains
to be verified.

## Related issue number

Observed in [extension
CI](https://github.com/alibaba/neug/actions/runs/34305275798/job/102322246010?pr=1039).
The same missing dependency exists on main; this fix is submitted
separately from #1039.
@liulx20 liulx20 closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant