sarutak commented on PR #59520: URL: https://github.com/apache/spark/pull/59520#issuecomment-6694663098
A few questions about the SDK as a whole, not blocking this PR: 1. Formats that become hard to change: offsets are stored in the offset log as the raw `serde_json` encoding of `Offset`, so a later SDK cannot change that encoding without breaking checkpoints. Should the docs promise it? The README calls the versioned JSON envelope of the serialized state an implementation detail (messages use the same envelope); does that include a promise that it stays a self-describing format (so that `#[serde(untagged)]` or `flatten` keep working), or should connectors avoid serde features that need one? 2. Extending the trait set: a new kind of operation means a new associated type on `DataSource`, which every connector must add (stable Rust has no associated type defaults). Is the plan to accept that as a breaking SDK release, and could the README say so? 3. Distribution and versions: how should third parties depend on the crate (`publish = false`), and how do SDK versions map to the ABI version and Spark releases? The crate name `spark-native-source` also covers sinks; if it should change (for example, to match `spark_datasource`), it is easier before anyone depends on it. 4. Driver/executor build mismatch: the README asks to use the same library build on all nodes; installed libraries are not checksummed across nodes, and the envelope version only detects SDK format changes. Is a build fingerprint in the envelope worth it, or is the README note enough? 5. Contract docs: the header says that unplanned batch readers and the last stream writer of a query are released once they are no longer referenced, and that `read` may be called for the same partition at the same time in several attempts. The `ScanBuilder` rustdoc says an unplanned builder is dropped; could the rustdoc of `ScanBuilder`, `StreamingWrite` and `PartitionReaderFactory::open` also say that the drop can happen late and on another thread, and that reads of a partition can overlap, so that connectors do not release exclusive resources only in `Drop`? 6. Decimal values: the header notes that a native scan written to a native sink copies decimals without a precision check. Should the SDK validate decimal values on read, given the O(n) cost? -- 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]
