Vamsi-klu commented on PR #58191:
URL: https://github.com/apache/spark/pull/58191#issuecomment-5406736584
Sorry about the last push. You asked for a design discussion and I treated
the
LSP notes as a patch spec.
What I think you meant:
SimpleInputPartition is not an InputPartition. InputPartition is a picklable
work split whose payload is .value. The wrapper used a subclass with
start/end
(the offset range for replay) and then narrowed read() to that subclass. That
is why the type: ignore[override] existed. cast and the ignore are both
covering
that up.
The wrapper should stay a DataSourceStreamReader. read() should keep the ABC
signature. The offset range should be the payload of a real InputPartition.
Proposed shape:
partitions(start, end) -> [InputPartition((start, end))]
read(partition: InputPartition) ->
simple_reader.readBetweenOffsets(*partition.value)
Delete SimpleInputPartition. No DATA_SOURCE_TYPE_MISMATCH. A wrong payload
here
is a Spark bug on the recovery path (executor unpickle in
plan_data_source_read), not a user type error.
I would not drop wrapper inheritance: once read accepts InputPartition, the
stronger-precondition issue is gone at the type level. Every stream reader
only
understands partitions it produced. That is already the ABC contract.
I would not change the public InputPartition subclassing docs or user sources
(RangePartition / BlockPartition). That pattern is documented for user data
sources. This is only the private adapter.
I will wait on this before touching more code.
cc @HyukjinKwon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
