Skip to content

Commit 373856e

Browse files
authored
fix(polling_source): eliminate per-batch schema inference, fix zero-row crash (ENG-952) (#261)
* docs(specs): add ENG-952 design spec for PollingSource zero-row batch fix * docs(specs): update ENG-952 spec — canonical-schema approach replaces Fix B * docs(plans): add ENG-952 implementation plan * test(polling_source): add failing regression tests for ENG-952 zero-row batch crash * refactor(test_polling_source): remove extraneous to_config/from_config from inline test impls Follow the established inline-class pattern used throughout the file: test impl classes define only identity, schema, poll, fetch, and close. Also add a descriptive failure message to the _accumulated_stream assertion so failures self-describe rather than raising an AttributeError on the next line. * fix(polling_source): establish canonical Arrow schema once, eliminating zero-row crash * fix(polling_source): defer infer-once schema establishment until first non-empty batch * docs(design_issues): add PS4 entry for ENG-952 zero-row batch schema crash * docs(polling_source): improve _build_stream_from_df docstring and remove stale type: ignore * fix(test_polling_source): update test_zero_row_batch_is_not_accumulated for _batches API ITL-617 replaced _accumulated_stream with an append-only _batches list. Zero-row batches produce empty ArrowTableStream entries in _batches, so the assertion must count total rows across all batches rather than batch-list length. Also rename DESIGN_ISSUES PS4 entry to PS5 to avoid collision with the ITL-617 PS4 entry that was added to main. --------- Co-authored-by: agent-kurodo[bot] <268466204+agent-kurodo[bot]@users.noreply.github.com>
1 parent 55d0439 commit 373856e

5 files changed

Lines changed: 1052 additions & 3 deletions

File tree

DESIGN_ISSUES.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,24 @@ the async loop loses the commit race. The lock is never held across ``await``.
172172

173173
---
174174

175+
### PS5 — `PollingSource` re-infers Arrow schema nullability per batch, crashing on zero-row polls
176+
**Status:** resolved
177+
**Severity:** high
178+
**Issue:** ENG-952
179+
180+
`_build_stream_from_df` called `infer_schema_nullable` on every batch. A zero-row batch has
181+
`null_count == 0` for all columns, so every field was inferred non-nullable.
182+
`_validate_combining_schemas` then rejected the batch against the accumulated stream's
183+
nullable schema.
184+
185+
**Fix:** `_build_stream_from_df` now establishes a `_canonical_arrow_schema` exactly once —
186+
from `impl.schema()` when declared (no inference, no warning), or from the first non-empty
187+
batch otherwise (with a `WARNING`-level log). All subsequent batches are cast to the canonical
188+
schema by column name. Per-batch nullability inference is eliminated. Zero-row frames before
189+
canonical schema establishment are skipped on the infer-once path.
190+
191+
---
192+
175193
## `src/orcapod/core/nodes/function_node.py`
176194

177195
### FN1 — `FunctionNodeBase.as_table()` returned empty schema when no data existed

src/orcapod/core/sources/polling_source.py

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ def __init__(
258258
self._cursor: Cursor[T] | None = None
259259
self._batches: list[ArrowTableStream] = []
260260
self._state_lock: threading.Lock = threading.Lock()
261+
self._canonical_arrow_schema: pa.Schema | None = None
261262
# Derive source_id from impl identity if not explicitly provided
262263
if self._source_id is None:
263264
self._source_id = str(self._impl.identity())
@@ -613,8 +614,13 @@ def _try_build_stream(self, data: FrameInitTypes) -> ArrowTableStream | None:
613614
return None
614615
return self._build_stream_from_df(df)
615616

616-
def _build_stream_from_df(self, df: pl.DataFrame) -> ArrowTableStream:
617-
"""Build an ``ArrowTableStream`` from a Polars DataFrame."""
617+
def _build_stream_from_df(self, df: pl.DataFrame) -> ArrowTableStream | None:
618+
"""Build an ``ArrowTableStream`` from a Polars DataFrame.
619+
620+
Returns ``None`` on the infer-once path when the batch has zero rows and
621+
no canonical schema has been established yet — the frame is silently
622+
skipped so that a spurious all-non-nullable schema is never recorded.
623+
"""
618624
from orcapod.core.streams.arrow_table_stream import ArrowTableStream
619625

620626
# Handle Object-dtype columns (same pattern as DataFrameSource)
@@ -628,7 +634,42 @@ def _build_stream_from_df(self, df: pl.DataFrame) -> ArrowTableStream:
628634
df = polars_data_utils.drop_system_columns(df)
629635

630636
arrow_table = df.to_arrow()
631-
arrow_table = arrow_table.cast(arrow_utils.infer_schema_nullable(arrow_table))
637+
638+
# Establish canonical schema on first call; apply it on every call.
639+
if self._canonical_arrow_schema is None:
640+
if self._tag_schema is not None and self._data_schema is not None:
641+
# Declared-schema path: derive Arrow schema from declared Python types.
642+
# T | None → nullable=True; plain T → nullable=False. No inference.
643+
combined = {**dict(self._tag_schema), **dict(self._data_schema)}
644+
self._canonical_arrow_schema = (
645+
self.data_context.type_converter.python_schema_to_arrow_schema(combined)
646+
)
647+
else:
648+
# Infer-once path: first non-empty batch establishes canonical nullability.
649+
# Skip zero-row tables: null_count is always 0 for empty tables, so
650+
# inference would set every field nullable=False — the original ENG-952 bug.
651+
if arrow_table.num_rows > 0:
652+
logger.warning(
653+
"PollingSource %r: no schema declared via impl.schema(); "
654+
"inferring nullability from first batch. Implement impl.schema() "
655+
"to avoid schema drift on zero-row polls or null-free batches.",
656+
self._source_id,
657+
)
658+
self._canonical_arrow_schema = arrow_utils.infer_schema_nullable(arrow_table)
659+
660+
# If _canonical_arrow_schema is still None here, this is a zero-row frame
661+
# on the infer-once path before any real data has arrived. Skip it —
662+
# the caller (_try_build_stream) will return None and the frame is ignored.
663+
if self._canonical_arrow_schema is None:
664+
return None
665+
666+
# Apply canonical nullability by column name (order-safe).
667+
canonical_nullable = {f.name: f.nullable for f in self._canonical_arrow_schema}
668+
target_schema = pa.schema([
669+
pa.field(f.name, f.type, nullable=canonical_nullable.get(f.name, f.nullable))
670+
for f in arrow_table.schema
671+
])
672+
arrow_table = arrow_table.cast(target_schema)
632673

633674
builder = SourceStreamBuilder(self.data_context, self.orcapod_config)
634675
result = builder.build(

0 commit comments

Comments
 (0)