You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/05/30 19:03:28 UTC

[GitHub] [arrow-rs] alamb opened a new pull request, #4313: Use `page_size` consistently, deprecate `pagesize`

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

   # Which issue does this PR close?
   N/A
   
   # Rationale for this change
   
   While working on https://github.com/influxdata/influxdb_iox/pull/7880 I noticed that the notion of page size was referred to with different terms:
   1.  `pagesize`: https://github.com/search?q=repo%3Aapache%2Farrow-rs%20pagesize&type=code
   2. `page_size`: https://github.com/search?q=repo%3Aapache%2Farrow-rs+page_size&type=code
   
   # What changes are included in this PR?
   
   Change all apis to use `page_size`, and deprecate APIs that refer to `pagesize`
   
   # Are there any user-facing changes?
   
   More consistent names, and some deprecated functions


-- 
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

[GitHub] [arrow-rs] tustvold merged pull request #4313: Use `page_size` consistently, deprecate `pagesize` in parquet WriterProperties

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


-- 
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

[GitHub] [arrow-rs] alamb commented on a diff in pull request #4313: Use `page_size` consistently, deprecate `pagesize`

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


##########
parquet/src/file/properties.rs:
##########
@@ -152,15 +152,33 @@ impl WriterProperties {
     /// Returns data page size limit.
     ///
     /// Note: this is a best effort limit based on the write batch size
+    #[deprecated(since = "41.0.0", note = "Use data_page_size_limit")]
     pub fn data_pagesize_limit(&self) -> usize {

Review Comment:
   What was especially annoying was the difference between `data_page_row_count_limit` and `datapage_size_limit` on the same properties module



-- 
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

[GitHub] [arrow-rs] alamb commented on a diff in pull request #4313: Use `page_size` consistently, deprecate `pagesize` in parquet WriterProperties

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


##########
parquet/src/file/properties.rs:
##########
@@ -351,16 +370,32 @@ impl WriterPropertiesBuilder {
     ///
     /// Note: this is a best effort limit based on value of
     /// [`set_write_batch_size`](Self::set_write_batch_size).
+    #[deprecated(since = "41.0.0", note = "Use set_dictionary_page_size_limit")]

Review Comment:
   Thank you @viirya 



-- 
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

[GitHub] [arrow-rs] viirya commented on a diff in pull request #4313: Use `page_size` consistently, deprecate `pagesize` in parquet WriterProperties

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


##########
parquet/src/file/properties.rs:
##########
@@ -351,16 +370,32 @@ impl WriterPropertiesBuilder {
     ///
     /// Note: this is a best effort limit based on value of
     /// [`set_write_batch_size`](Self::set_write_batch_size).
+    #[deprecated(since = "41.0.0", note = "Use set_dictionary_page_size_limit")]

Review Comment:
   ```suggestion
       #[deprecated(since = "41.0.0", note = "Use set_data_page_size_limit")]
   ```



-- 
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