You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "westonpace (via GitHub)" <gi...@apache.org> on 2023/02/02 21:36:38 UTC
[GitHub] [arrow] westonpace opened a new pull request, #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
westonpace opened a new pull request, #34013:
URL: https://github.com/apache/arrow/pull/34013
Also cleans up the Acero server example to use the DeclarationToXyz methods
--
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
[GitHub] [arrow] westonpace commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1416367111
> https://github.com/apache/arrow/pull/33917 provides an immediate fix for https://github.com/apache/arrow/issues/33960. I unlinked this from https://github.com/apache/arrow/issues/33960 since it does not. Nope, the bots wouldn't let me do that without changing the PR title, I think.
I thought the merge script would allow me to merge this without updating the issue but it seems to have closed it anyway (I suppose GH does that automatically). I reopened #33960 manually.
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1415977158
* Closes: #33960
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1414406749
:warning: GitHub issue #33960 **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
[GitHub] [arrow] lidavidm commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1416370583
In this case, the PR title and description still reference that issue and the sidebar notes that the issue is linked, so yeah, it seems GitHub did that
--
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
[GitHub] [arrow] ianmcook commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "ianmcook (via GitHub)" <gi...@apache.org>.
ianmcook commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1415977374
#33917 provides an immediate fix for #33960. I unlinked this from #33917 since it does not.
--
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
[GitHub] [arrow] westonpace commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1414406713
I'm not sure if this fully closes #33960 but it was inspired by it.
--
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
[GitHub] [arrow] westonpace merged pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace merged PR #34013:
URL: https://github.com/apache/arrow/pull/34013
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1414406692
* Closes: #33960
--
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
[GitHub] [arrow] paleolimbot commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1414695012
I should have done a search for "Declaration" before somewhat approving this approach 😬
I think this *would* close #33960 if we were constructing our plan in a non-deprecated way:
https://github.com/apache/arrow/blob/2ad89525173bbe3e03814eb4310b6e58fad5b989/r/src/compute-exec.cpp#L44-L46
Since I don't see any declarations in our plan-building process, I'm not sure we can avoid building an entire exec plan to get the schema.
It does help us with our Substrait schema calculation, though, since presumably we could add a substrait-plan-in-schema-out method.
https://github.com/apache/arrow/blob/2ad89525173bbe3e03814eb4310b6e58fad5b989/r/src/compute-exec.cpp#L530-L532
--
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
[GitHub] [arrow] ursabot commented on pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34013:
URL: https://github.com/apache/arrow/pull/34013#issuecomment-1416668120
Benchmark runs are scheduled for baseline = 3be5e11687ea874b7293d17227943bd8244e8789 and contender = 54eedb95ec504a715d557e71139ae4df9657fde6. 54eedb95ec504a715d557e71139ae4df9657fde6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:25.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/778ab7898d9144e5991aa2db2e664089...1fe4e173f2d249cd83b04134b10845f1/)
[Failed :arrow_down:0.89% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/741f7d90310b476ba0b3ef835b1bd8ba...cf38c4686b2e4e6ba7b9895f42434703/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a005f0c9e2804f30a6996888dd0e5008...5bce3d8e3cbf445a838ab012dbe4cb11/)
[Finished :arrow_down:1.08% :arrow_up:0.03%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/dd4dbf57262a4b1a959916516d547b54...ef1d5d6e055642ab98c306bfbe40d326/)
Buildkite builds:
[Finished] [`54eedb95` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2321)
[Finished] [`54eedb95` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2348)
[Finished] [`54eedb95` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2318)
[Finished] [`54eedb95` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2339)
[Finished] [`3be5e116` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2320)
[Failed] [`3be5e116` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2347)
[Finished] [`3be5e116` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2319)
[Finished] [`3be5e116` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2338)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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
[GitHub] [arrow] westonpace commented on a diff in pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on code in PR #34013:
URL: https://github.com/apache/arrow/pull/34013#discussion_r1095250885
##########
cpp/src/arrow/compute/exec/exec_plan.cc:
##########
@@ -822,6 +868,10 @@ Result<std::unique_ptr<RecordBatchReader>> DeclarationToReader(
}
Status Close() override {
+ if (!!iterator_) {
Review Comment:
:cold_sweat: yes. Fixed.
--
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
[GitHub] [arrow] lidavidm commented on a diff in pull request #34013: GH-33960: [C++] Add DeclarationToSchema and DeclarationToString helper methods.
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #34013:
URL: https://github.com/apache/arrow/pull/34013#discussion_r1095114955
##########
cpp/src/arrow/compute/exec/exec_plan.cc:
##########
@@ -822,6 +868,10 @@ Result<std::unique_ptr<RecordBatchReader>> DeclarationToReader(
}
Status Close() override {
+ if (!!iterator_) {
Review Comment:
Should this just be `!iterator_`?
--
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