Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add an example for RecordBatchReceiverStreamBuilder #8888

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

SteveLauC
Copy link
Contributor

Which issue does this PR close?

No

Rationale for this change

I was quite confused about how to this type when I first saw it, so let's add an example to make it more clear on its usage.

What changes are included in this PR?

  1. Add an example for RecordBatchReceiverStreamBuilder
  2. Some other minor changes that may improve the doc

Are these changes tested?

Yes, tested with cargo t

Are there any user-facing changes?

No

@@ -156,14 +156,62 @@ impl<O: Send + 'static> ReceiverStreamBuilder<O> {
}
}

/// Builder for [`RecordBatchReceiverStream`] that propagates errors
/// Builder for `RecordBatchReceiverStream` that propagates errors
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This [] is removed because there is no item named RecordBatchReceiverStream, the RecordBatchReceiverStreamBuilder::build() will return a trait object rather than a concrete type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I just realized that this item exists, it is just hidden from the documents:

#[doc(hidden)]
pub struct RecordBatchReceiverStream {}

@@ -209,7 +259,7 @@ impl RecordBatchReceiverStreamBuilder {
self.inner.spawn_blocking(f)
}

/// runs the input_partition of the `input` ExecutionPlan on the
/// runs the `partition` of the `input` ExecutionPlan on the
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to match the argument name of this run_input() function

Copy link
Member

@Ted-Jiang Ted-Jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alamb alamb added the documentation Improvements or additions to documentation label Jan 18, 2024
@alamb
Copy link
Contributor

alamb commented Jan 18, 2024

Thank you for this @SteveLauC -- it is really appreciated that you are making it easier for others to use DataFusion based on experience / rough edges you encounter.

@alamb alamb merged commit eb9bbe8 into apache:main Jan 18, 2024
22 checks passed
@SteveLauC SteveLauC deleted the physical_plan_stream branch January 18, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants