rustyconover commented on PR #389: URL: https://github.com/apache/arrow-js/pull/389#issuecomment-4005059993
Hi @trxcllnt and @kou, I think my original scope was too big so I'm going to scope this down to just the core issue that took a few hours of debugging. So based on your feedback I: - **Removed redundant tests** — dropped the `autoDestroy=false` multi-stream test (already covered by `stream-writer-tests.ts`, `streams-dom-tests.ts`, and `streams-node-tests.ts`) and the closed-writer test (pre-existing behavior, not introduced by this PR) - **Added JSDoc for the new throwing behavior** on `autoDestroy` - **Switched to static imports** for consistency with other test files The PR is now scoped to just the core fix: when `autoDestroy=true` (the default), writing a batch with a mismatched schema throws an error instead of silently closing the writer and dropping the batch. **Why throw instead of silently closing:** The previous behavior called `return this.close()` when it detected a schema mismatch with `autoDestroy=true`. From the caller's perspective, `writer.write(batch)` returned successfully — there was no indication that the batch was dropped and the writer was closed. This made schema mismatches extremely difficult to debug, since data silently disappeared with no error, no warning, and no way to distinguish a successful write from a dropped one. Throwing makes the problem immediately visible at the point where it occurs. It may be that the user should check if they are trying to write mismatched schemas, but if you use PyArrow or the other Arrow implementations they raise an exception when you try to write a record batch that doesn't match the schema. I'm trying to promote consistent behavior. -- 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]
