BsoBird commented on code in PR #2717:
URL: https://github.com/apache/orc/pull/2717#discussion_r4258239169
##########
java/core/src/java/org/apache/orc/impl/reader/StripePlanner.java:
##########
@@ -140,7 +140,22 @@ public StripePlanner(StripePlanner old) {
*/
public StripePlanner parseStripe(StripeInformation stripe,
boolean[] columnInclude) throws IOException
{
- OrcProto.StripeFooter footer = dataReader.readStripeFooter(stripe);
+ return parseStripe(stripe, columnInclude,
dataReader.readStripeFooter(stripe));
+ }
+
+ /**
+ * Parse a new stripe using a footer that was already read. Resets the
+ * current stripe state. This avoids a redundant {@code readStripeFooter}
+ * call when the caller (e.g. {@code RecordReaderImpl}) has already read
+ * the footer for the same stripe.
+ * @param stripe the new stripe
+ * @param columnInclude an array with true for each column to read
+ * @param footer the stripe footer that was already read for {@code stripe}
+ * @return this for method chaining
+ */
+ public StripePlanner parseStripe(StripeInformation stripe,
+ boolean[] columnInclude,
+ OrcProto.StripeFooter footer) throws
IOException {
Review Comment:
Thanks for the suggestion — I think we should keep the new three-arg
overload declared with throws IOException.
Although the 3-arg overload doesn't itself call DataReader.read* today, it
performs parsing work that can surface I/O-related failures (and may in
future). Keeping the checked IOException:
- preserves the existing error-handling contract and avoids surprising
callers;
- keeps the two-arg wrapper and callers simple (no extra catch/wrap logic
required);
- future-proofs the API in case the implementation later needs I/O or
underlying helpers throw IOException.
If we want to remove the checked exception, we should do that deliberately
and consistently (for example by converting IOExceptions to
UncheckedIOException in one place and updating call sites and documentation). I
don't recommend silently narrowing the overload's declared exceptions in this
patch.
--
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]
