ARROW-14074: [C++][Compute] C++ consumer of compute IR #11384
Conversation
bkietz
force-pushed
the
14074-Sketch-a-C-consumer-of-co
branch
2 times, most recently
from
d04021b
to
1d4c37e
Compare
bkietz
force-pushed
the
14074-Sketch-a-C-consumer-of-co
branch
from
4ac1d95
to
9c9d62e
Compare
| @@ -302,7 +302,7 @@ struct TemporalScalar : internal::PrimitiveScalar<T> { | |||
| using internal::PrimitiveScalar<T>::PrimitiveScalar; | |||
| using ValueType = typename TemporalScalar<T>::ValueType; | |||
|
|
|||
| explicit TemporalScalar(ValueType value, std::shared_ptr<DataType> type) | |||
| TemporalScalar(ValueType value, std::shared_ptr<DataType> type) | |||
There was a problem hiding this comment.
Probably just my seriously rusty C++, but is this to allow copy-list initialization somewhere else?
There was a problem hiding this comment.
If the constructor could accept a single argument, explicit would allow implicit conversion to TemporalScalar. Since the constructor requires two arguments the keyword is fully redundant (which clang-tidy nagged me about when I opened the file, prompting the edit)
cpp/src/arrow/util/io_util.cc
Outdated
| @@ -578,6 +607,19 @@ Result<std::vector<PlatformFilename>> ListDir(const PlatformFilename& dir_path) | |||
|
|
|||
| #endif | |||
|
|
|||
| Status SetWorkingDir(const PlatformFilename& dir_path) { | |||
There was a problem hiding this comment.
Setting the working directory is a fraught in the presence of multiple threads.
You can almost always accomplish what you need to do after running chdir in a thread-safe way without chdir.
So, with that in mind: why is this function necessary?
There was a problem hiding this comment.
It can be removed, will do
There was a problem hiding this comment.
FTR, I used this because flatc always outputs the binary representation to the current working directory (it will not accept the -o option) and it seemed neater to ensure we were always writing that to the temporary directory we allocated
|
@cpcloud I think I've addressed all your comments, is this good to merge? |
|
Benchmark runs are scheduled for baseline = dce1415 and contender = 528625e. 528625e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Provide a consumer for compute IR.
The consumer produces
Declarationobjects from buffers of Compute IR. This allows us to specify mappings from IR to ExecNode graphs without needing those specific nodes in the registry or requiring the registered factories to support all features declared in IR.This is tested by using the flatbuffer compiler to convert JSON into the corresponding binary representation.