GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test#47318
GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test#47318kou merged 2 commits intoapache:mainfrom
Conversation
|
|
`gandiva::Cache` requires pointer type for value type.
|
Passed with GCC 12: https://github.com/apache/arrow/actions/runs/16926330455/job/47962616526?pr=47318#step:6:4091 I'll update to Debian 13 from 12 to use more recent GCC. |
|
I think the Meson failure will require #47041 to pass |
|
Hmm. It seems that newer (unreleased RapidJSON) is installed by conda: https://github.com/apache/arrow/actions/runs/16934205748/job/47986801327?pr=47318#step:6:346 but it's not detected by Meson: https://github.com/apache/arrow/actions/runs/16934205748/job/47986801327?pr=47318#step:6:502 I think that it's a problem. Anyway, let's work on it as a separated task. I'll merge this because I could confirm that this PR can fix GCC 12 + C++23 problem and GCC 14 + C++23 has another problems. |
|
Ah cool. I didn't realize conda patched RapidJSON like that. I guess the problem is that Meson is using pkg-config to search for the dependency, and being header-only there is no .pc file distributed with it (I think this is intentional, but if not I don't see a .pc file when installing locally via conda). Per Meson's dependency detection logic, I wonder if we just need to be more clear that rapidjson should be detected as a system dependency, and let it search the PATH for the header: https://mesonbuild.com/Dependencies.html#dependency-detection-method |
|
It seems that conda's rapidjson package https://anaconda.org/conda-forge/rapidjson includes $ find . -name '*.pc'
./lib/pkgconfig/RapidJSON.pcIt seems that we need to use |
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 6660e84. There weren't enough matching historic benchmark results to make a call on whether there were regressions. The full Conbench report has more details. |
Rationale for this change
gandiva::Cacherequires pointer type for value type.What changes are included in this PR?
Use
std::shared_ptr<std::string>notstd::stringforValueType.Are these changes tested?
Yes.
Are there any user-facing changes?
No.