You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2025/08/13 02:48:57 UTC
[PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
kou opened a new pull request, #47318:
URL: https://github.com/apache/arrow/pull/47318
### Rationale for this change
`gandiva::Cache` requires pointer type for value type.
### What changes are included in this PR?
Use `std::shared_ptr<std::string>` not `std::string` for `ValueType`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3182006405
:warning: GitHub issue #47317 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3183009207
Passed with GCC 12:
https://github.com/apache/arrow/actions/runs/16926330455/job/47962616526?pr=47318#step:6:4091
```text
100% tests passed, 0 tests failed out of 106
```
I'll update to Debian 13 from 12 to use more recent GCC.
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3186600418
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
```text
rapidjson 1.1.0.post20240409 h3f2d84a_2 conda-forge
```
but it's not detected by Meson:
https://github.com/apache/arrow/actions/runs/16934205748/job/47986801327?pr=47318#step:6:502
```text
Run-time dependency rapidjson found: NO (tried pkgconfig)
```
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.
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3186716311
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
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #47318:
URL: https://github.com/apache/arrow/pull/47318
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3186851513
It seems that conda's rapidjson package https://anaconda.org/conda-forge/rapidjson includes `RapidJSON.pc`:
https://anaconda.org/conda-forge/rapidjson/1.1.0.post20240409/download/linux-64/rapidjson-1.1.0.post20240409-h3f2d84a_2.conda
```console
$ find . -name '*.pc'
./lib/pkgconfig/RapidJSON.pc
```
It seems that we need to use `dependency('RapidJSON')` not `dependency('rapidjson')`.
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3184149026
I think the Meson failure will require https://github.com/apache/arrow/pull/47041 to pass
--
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
Re: [PR] GH-47317: [C++][C++23][Gandiva] Use pointer for Cache test [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #47318:
URL: https://github.com/apache/arrow/pull/47318#issuecomment-3187831963
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 6660e848761d9e51ed75df91bbf309d6c0af6af2.
There weren't enough matching historic benchmark results to make a call on whether there were regressions.
The [full Conbench report](https://github.com/apache/arrow/runs/48076121572) has more details.
--
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