Minor: Improve docstrings on WriterPropertiesBuilder #3068
Conversation
| /// Sets write batch size | ||
| /// Sets write batch size. | ||
| /// | ||
| /// For performance reasons, data for each column is written in |
There was a problem hiding this comment.
I am pretty sure this is accurate but it would be nice for someone else to confirm
There was a problem hiding this comment.
Verified it by looking at the related code and this is accurate.
| @@ -306,38 +306,46 @@ impl WriterPropertiesBuilder { | |||
| self | |||
| } | |||
|
|
|||
| /// Sets best effort maximum size of a data page in bytes | |||
| /// Sets best effort maximum size of a data page in bytes. | |||
There was a problem hiding this comment.
I added . as well to make the docstrings for these properties consistent with the others in this file
|
Benchmark runs are scheduled for baseline = 5a3ecc2 and contender = e4e15f8. e4e15f8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
@alamb Thanks 👍 |
Which issue does this PR close?
N/A
Rationale for this change
Even though the docs did mention the fact that the limits were a function of the write batch size, I think both @Ted-Jiang and I missed the subtlety initially. See conversation on apache/datafusion#4131 (comment)
What changes are included in this PR?
Add some additional commentary on write batch size docs as well as backlinks from other limits
Are there any user-facing changes?
Only docstrings