nealrichardson commented on PR #41998: URL: https://github.com/apache/arrow/pull/41998#issuecomment-2275897449
> Sorry @HaochengLIU, I think I forgot to save my review. You should see them now. > > @thisisnic is right about my note. See this example: > > https://github.com/apache/arrow/blob/c69c1d80eb27022c1b3f91fbbc7979f8293c9e50/r/src/extension-impl.cpp#L103 > > I think here we want to do here is (1) conditionally define two versions of `fs___S3FileSystem__create`, one that supports `check_directory_existence_before_creation` and one that doesn't and then (2) catch calls to `S3FileSystem$create` which pass the option `check_directory_existence_before_creation` when `arrow::arrow_info()$build_info$cpp_version < "17.0.0"` and probably emit a warning but otherwise continue. Let me know if this isn't clear. The tricky thing about that is the codegen script that creates `arrowExports.cpp`: it doesn't know how to handle the conditional definitions like that, and I'm not sure it's worth extending to do so. Another option would be to refactor to send a list of options to `fs___S3FileSystem__create` (and the other FileSystem create methods, for consistency) and then pull the settings out of the list inside the C++ function, like how `make_compute_options()` works (https://github.com/apache/arrow/blob/main/r/src/compute.cpp#L129). Then the `#if ARROW_VERSION_MAJOR >= 16` would just be wrapping a few lines inside that function and not affecting the signature. -- 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]
