paleolimbot commented on code in PR #36992:
URL: https://github.com/apache/arrow/pull/36992#discussion_r1289449391
##########
r/tests/testthat/test-parquet.R:
##########
@@ -484,3 +484,30 @@ test_that("Can read Parquet files from a URL", {
expect_true(tibble::is_tibble(pu))
expect_identical(dim(pu), c(10L, 11L))
})
+
+test_that("thrift string size can be specified when reading Parquet files", {
+ tf <- tempfile()
+ on.exit(unlink(tf))
+
+ table <- arrow_table(example_data)
+
+ write_parquet(table, tf)
+
+ reader_props <- ParquetReaderProperties$create()
+ reader_props$set_thrift_string_size_limit(1)
+
+ expect_identical(reader_props$thrift_string_size_limit(), 1L)
+
+ file <- make_readable_file(tf)
+ on.exit(file$close())
+
+ # We get an error if we set the Thrift string size limit too small
+ expect_error(ParquetFileReader$create(file, reader_props = reader_props))
Review Comment:
I agree that it's not ideal to test an error message from C++; however, we
do it a lot and I think it's much much better than `expect_error()` without a
message (which can swallow unrelated errors or even completely invalid code).
`expect_error()` without any constraining information has bitten me a number of
times and our existing tests that use this pattern don't seem to cause us
maintenance effort.
--
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]
