You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Ox0400 (via GitHub)" <gi...@apache.org> on 2023/11/04 12:11:29 UTC
[PR] New an env ARROW_JSON_MAX_NUM_ROWS to overwrite default kMaxParserNum… [arrow]
Ox0400 opened a new pull request, #38582:
URL: https://github.com/apache/arrow/pull/38582
https://github.com/apache/arrow/issues/28994
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on how
to contribute here:
* [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
-->
### Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
### What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
### Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
### Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
-->
<!-- **This PR includes breaking changes to public APIs.** -->
<!--
Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1388137114
##########
cpp/src/arrow/json/parser.h:
##########
@@ -56,7 +56,8 @@ struct Kind {
static Status ForType(const DataType& type, Kind::type* kind);
};
-constexpr int32_t kMaxParserNumRows = 100000;
+/// Set the max parse row is 2147483647.
+constexpr int32_t kMaxParserNumRows = std::numeric_limits<int32_t>::max();
Review Comment:
IMO, it would be slightly more readable to use the limit directly below and modify the error return from "Exceeded maximum rows" to "Row count overflowed int32_t"
```suggestion
```
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1813588943
> Most of the CI failures do seem unrelated.
>
> However, could you ensure that the lint/format job passes? Also, please update the PR description since that's what winds up in the git log
Yes, am think so. The PR title & desc already updated.
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1828605442
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit ca4655764900f3e216d4d0a9586a03b78dee7f01.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/19071426654) 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
Re: [PR] GH-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1803054483
> I'm not sure what the application is for this option in JSON parsing, and I'd lean toward removing it rather than making it configurable unless/until that capability is necessary to someone. (Unless I missed something, discussion on the issue did not reveal a use case for restricting json rows parsed per batch)
🤔 You are right, will delete the limit for parse json.
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1793568894
:warning: GitHub issue #28994 **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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1803864334
Hi @bkietz, code has update, plz review.
I change the default max row to `int_32::max()` to make sure the loop can be exit.
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1388815701
##########
cpp/src/arrow/json/parser.h:
##########
@@ -56,7 +56,8 @@ struct Kind {
static Status ForType(const DataType& type, Kind::type* kind);
};
-constexpr int32_t kMaxParserNumRows = 100000;
+/// Set the max parse row is 2147483647.
+constexpr int32_t kMaxParserNumRows = std::numeric_limits<int32_t>::max();
Review Comment:
Hi @bkietz, thanks for your review. The error msg has update.
I personally prefer to define the result of `int32_t::max()` as a constant in `parse.h` to be consistent with other constants.
Do you suggest moving it to `parse.cc` or direct using it like `for (; num_rows_ < [kMaxParserNumRows](std::numeric_limits<int32_t>::max()); ++num_rows_) {}` on `parse.cc` ?
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1386961772
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -46,6 +46,10 @@ namespace arrow {
using internal::BitsetStack;
using internal::checked_cast;
+static const char* kEnvJsonMaxNumRows = "ARROW_JSON_MAX_NUM_ROWS";
Review Comment:
If this must be configurable, we should add it to ReadOptions or ParseOptions instead of exposing an environment variable.
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz merged PR #38582:
URL: https://github.com/apache/arrow/pull/38582
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1815666093
> There's still a lingering python formatting error, then I think we can merge this https://github.com/apache/arrow/actions/runs/6858477451/job/18745903676#step:5:905
Has recommitted the test code, plz review again.
--
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] New an env ARROW_JSON_MAX_NUM_ROWS to overwrite default kMaxParserNum… [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1793426692
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1397741594
##########
python/pyarrow/tests/test_json.py:
##########
@@ -17,6 +17,7 @@
from collections import OrderedDict
from decimal import Decimal
+import os
Review Comment:
Looks like the linter is complaining about this unused import.
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1398637010
##########
python/pyarrow/tests/test_json.py:
##########
@@ -17,6 +17,7 @@
from collections import OrderedDict
from decimal import Decimal
+import os
Review Comment:
Hi @bkietz, fixed, plz try again, I have run `archery lint --python` on local and all passed.
```
➜ python git:(main) ✗ archery lint --python
INFO:archery:Running Python formatter (autopep8)
INFO:archery:Running Python linter (flake8)
INFO:archery:Running Cython linter (cython-lint)
➜ python git:(main) ✗
```
##########
python/pyarrow/tests/test_json.py:
##########
@@ -17,6 +17,7 @@
from collections import OrderedDict
from decimal import Decimal
+import os
Review Comment:
Hi @bkietz, fixed, plz try again, I have run `archery lint --python` on local and all passed.
```
➜ python git:(main) ✗ archery lint --python
INFO:archery:Running Python formatter (autopep8)
INFO:archery:Running Python linter (flake8)
INFO:archery:Running Cython linter (cython-lint)
➜ python git:(main) ✗
```
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1386961772
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -46,6 +46,10 @@ namespace arrow {
using internal::BitsetStack;
using internal::checked_cast;
+static const char* kEnvJsonMaxNumRows = "ARROW_JSON_MAX_NUM_ROWS";
Review Comment:
Instead of exposing this as an environment variable, please add it to ReadOptions or ParseOptions.
##########
python/pyarrow/tests/test_json.py:
##########
@@ -322,6 +323,30 @@ def test_stress_block_sizes(self):
# Better error output
assert table.to_pydict() == expected.to_pydict()
+ def test_max_num_rows(self):
+ data, expected = make_random_json(num_cols=2, num_rows=10)
+ table = self.read_bytes(data)
+ assert table.num_rows == 10
+ with pytest.raises(pa.ArrowInvalid, match="Exceeded maximum rows"):
Review Comment:
Instead of raising an error for exceeding the maximum number of rows, I think it'd be better to just skip the rest of the block. This is the behavior in CSV, which enables users to restrict expensive row processing to just a few rows in the case where only a few rows are required. If an error should be emitted instead that can be done after checking the number of rows against the configured limit.
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1388815701
##########
cpp/src/arrow/json/parser.h:
##########
@@ -56,7 +56,8 @@ struct Kind {
static Status ForType(const DataType& type, Kind::type* kind);
};
-constexpr int32_t kMaxParserNumRows = 100000;
+/// Set the max parse row is 2147483647.
+constexpr int32_t kMaxParserNumRows = std::numeric_limits<int32_t>::max();
Review Comment:
Hi @bkietz, thanks for your review. The error msg has update.
I personally prefer to define the result of `int32_t::max()` as a constant in `parse.h` to be consistent with other constants.
Do you suggest moving it to `parse.cc` or direct using it like `for (; num_rows_ < [kMaxParserNumRows](std::numeric_limits<int32_t>::max()); ++num_rows_) {}` ?
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1398631515
##########
python/pyarrow/tests/test_json.py:
##########
@@ -17,6 +17,7 @@
from collections import OrderedDict
from decimal import Decimal
+import os
Review Comment:
Oh, sorry ....
I try `python3.10 -m pip install -e 'dev/archery[all]'` raise this error `ERROR: dev/archery[all] is not a valid editable requirement. It should either be a path to a local project or a VCS URL (beginning with bzr+http, bzr+https, bzr+ssh, bzr+sftp, .....). ` ref: https://arrow.apache.org/docs/developers/python.html#coding-style
So I was run `pylint pyarrow/tests/test_json.py` on local not raise the error. I will remove the import.
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1398631515
##########
python/pyarrow/tests/test_json.py:
##########
@@ -17,6 +17,7 @@
from collections import OrderedDict
from decimal import Decimal
+import os
Review Comment:
Oh, sorry ....
I try `python3.10 -m pip install -e 'dev/archery[all]'` raise this error `ERROR: dev/archery[all] is not a valid editable requirement. It should either be a path to a local project or a VCS URL (beginning with bzr+http, bzr+https, bzr+ssh, bzr+sftp, .....). ` ref: https://arrow.apache.org/docs/developers/python.html#coding-style
So I was run `pylint pyarrow/tests/test_json.py` on local not raise the error. I will remove the import.
but `flake8 pyarrow/tests/test_json.py` can raise the error `pyarrow/tests/test_json.py:20:1: F401 'os' imported but unused`
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1391348218
##########
cpp/src/arrow/json/parser.h:
##########
@@ -56,7 +56,8 @@ struct Kind {
static Status ForType(const DataType& type, Kind::type* kind);
};
-constexpr int32_t kMaxParserNumRows = 100000;
+/// Set the max parse row is 2147483647.
+constexpr int32_t kMaxParserNumRows = std::numeric_limits<int32_t>::max();
Review Comment:
I think it's more readable with direct usage in this case; since the error message now explains that we are guarding against int32 overflow, we should make it clear inline that we are checking against int32's max. A named constant can be introduced when we're limiting against something configurable instead of the numeric limits of int32.
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1811734892
Hi @bkietz, the CI some jobs is failed, some fail job look like because losted git runners. `The hosted runner: GitHub Actions 389 lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.`
And one failed job it's same error with other PR CI job.
```
Testing C ArrowArray from file 'map_non_canonical'
-- Skipping test because producer Java does not support C ArrowArray
======================================================================
Fatal Python error: Segmentation fault
Thread 0x00007fa43ba76640 (most recent call first):
<no Python frame>
Thread 0x00007fa477307740 (most recent call first):
File "/arrow/dev/archery/archery/integration/tester_java.py", line 154 in export_batch_from_json
File "/arrow/dev/archery/archery/integration/runner.py", line 497 in do_run
File "/arrow/dev/archery/archery/integration/runner.py", line 520 in _run_c_array_test_cases
File "/arrow/dev/archery/archery/integration/runner.py", line 197 in case_wrapper
File "/arrow/dev/archery/archery/integration/runner.py", line 206 in _run_test_cases
File "/arrow/dev/archery/archery/integration/runner.py", line 435 in _compare_c_data_implementations
File "/arrow/dev/archery/archery/integration/runner.py", line 129 in run_c_data
File "/arrow/dev/archery/archery/integration/runner.py", line 638 in run_all_tests
File "/arrow/dev/archery/archery/cli.py", line 787 in integration
File "/opt/conda/envs/arrow/lib/python3.11/site-packages/click/core.py", line 783 in invoke
File "/opt/conda/envs/arrow/lib/python3.11/site-packages/click/core.py", line 1434 in invoke
File "/opt/conda/envs/arrow/lib/python3.11/site-packages/click/core.py", line 1688 in invoke
File "/opt/conda/envs/arrow/lib/python3.11/site-packages/click/core.py", line 1078 in main
File "/opt/conda/envs/arrow/lib/python3.11/site-packages/click/core.py", line 1157 in __call__
File "/opt/conda/envs/arrow/bin/archery", line 33 in <module>
Extension modules: work._ruamel_yaml, _ruamel_yaml, _cffi_backend, _brotli, markupsafe._speedups, pygit2._pygit2, numpy.core._multiarray_umath, numpy.core._multiarray_tests, numpy.linalg._umath_linalg, numpy.fft._pocketfft_internal, numpy.random._common, numpy.random.bit_generator, numpy.random._bounded_integers, numpy.random._mt19937, numpy.random.mtrand, numpy.random._philox, numpy.random._pcg64, numpy.random._sfc64, numpy.random._generator, _jpype (total: 20)
/arrow/ci/scripts/integration_arrow.sh: line 62: 13169 Segmentation fault (core dumped) archery integration --run-c-data --run-ipc --run-flight --with-cpp=$([ "$ARROW_INTEGRATION_CPP" == "ON" ] && echo "1" || echo "0") --with-csharp=$([ "$ARROW_INTEGRATION_CSHARP" == "ON" ] && echo "1" || echo "0") --with-go=$([ "$ARROW_INTEGRATION_GO" == "ON" ] && echo "1" || echo "0") --with-java=$([ "$ARROW_INTEGRATION_JAVA" == "ON" ] && echo "1" || echo "0") --with-js=$([ "$ARROW_INTEGRATION_JS" == "ON" ] && echo "1" || echo "0") --gold-dirs=$gold_dir/0.14.1 --gold-dirs=$gold_dir/0.17.1 --gold-dirs=$gold_dir/1.0.0-bigendian --gold-dirs=$gold_dir/1.0.0-littleendian --gold-dirs=$gold_dir/2.0.0-compression --gold-dirs=$gold_dir/4.0.0-shareddict
```
https://github.com/apache/arrow/actions/runs/6870662307/job/18685993426?pr=38721#step:8:17634
https://github.com/apache/arrow/actions/runs/6858477459/job/18674673496#step:8:17640
--
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-28994: [C++][JSON] Add support for customizing the max rows [arrow]
Posted by "Ox0400 (via GitHub)" <gi...@apache.org>.
Ox0400 commented on code in PR #38582:
URL: https://github.com/apache/arrow/pull/38582#discussion_r1392058004
##########
cpp/src/arrow/json/parser.h:
##########
@@ -56,7 +56,8 @@ struct Kind {
static Status ForType(const DataType& type, Kind::type* kind);
};
-constexpr int32_t kMaxParserNumRows = 100000;
+/// Set the max parse row is 2147483647.
+constexpr int32_t kMaxParserNumRows = std::numeric_limits<int32_t>::max();
Review Comment:
> I think it's more readable with direct usage in this case; since the error message now explains that we are guarding against int32 overflow, we should make it clear inline that we are checking against int32's max. A named constant can be introduced when we're limiting against something configurable instead of the numeric limits of int32.
Thank you for your explanation, I have removed the constant and now using it directly in the loop.
--
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-28994: [C++][JSON] Change the max rows to Unlimited(int_32) [arrow]
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on PR #38582:
URL: https://github.com/apache/arrow/pull/38582#issuecomment-1814427020
There's still a lingering python formatting error, then I think we can merge this https://github.com/apache/arrow/actions/runs/6858477451/job/18745903676#step:5:905
--
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