You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "cetra3 (via GitHub)" <gi...@apache.org> on 2025/07/03 02:21:26 UTC

[PR] Add `get_ref/get_mut` to JSON Writer [arrow-rs]

cetra3 opened a new pull request, #7854:
URL: https://github.com/apache/arrow-rs/pull/7854

   # Which issue does this PR close?
   
   None
   
   # Rationale for this change
   
   I need access to the writer so that I can flush an external buffer when bytes are written.
   
   # What changes are included in this PR?
   
   A couple of methods to the JSON writer.  These methods already exist on other writers
   
   # Are these changes tested?
   
   N/A
   
   # Are there any user-facing changes?
   
   Yes, a couple extra methods on the JSON writer.
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add `get_ref/get_mut` to JSON Writer [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7854:
URL: https://github.com/apache/arrow-rs/pull/7854#issuecomment-3038810719

   Thank you @cetra3 and @adriangb 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add `get_ref/get_mut` to JSON Writer [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7854:
URL: https://github.com/apache/arrow-rs/pull/7854


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add `get_ref/get_mut` to JSON Writer [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7854:
URL: https://github.com/apache/arrow-rs/pull/7854#discussion_r2183158631


##########
arrow-json/src/writer/mod.rs:
##########
@@ -413,6 +413,18 @@ where
         Ok(())
     }
 
+    /// Gets a reference to the underlying writer.
+    pub fn get_ref(&self) -> &W {
+        &self.writer
+    }
+
+    /// Gets a mutable reference to the underlying writer.
+    ///
+    /// It is inadvisable to directly write to the underlying writer.

Review Comment:
   I think this comment is confusing -- I imagine users would be wondering why is there an API that the documentation advises not to use
   
   Thus, I think it would be useful to explain **why** it is not advisable or remove it all together
   
   ```suggestion
       /// Gets a mutable reference to the underlying writer.
       ///
       /// Writing to the underlying writer must be done with care
       /// to avoid corrupting the output JSON. 
   ```
   
   I realize this is copy/paste from https://docs.rs/arrow-ipc/55.2.0/arrow_ipc/writer/struct.FileWriter.html#method.get_mut but I think JSON has fewer reasons that writing to the underlying write is problematic



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add `get_ref/get_mut` to JSON Writer [arrow-rs]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #7854:
URL: https://github.com/apache/arrow-rs/pull/7854#issuecomment-3032009698

   @alamb this is a minor simple change, as Peter said it's already on the IPC writer:
   
   https://github.com/apache/arrow-rs/blob/42bb5b419d1835a3250f78a26198f78fc89dadc5/arrow-ipc/src/writer.rs#L1116-L1126
   
   Could we get this merged?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org