You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/06 17:20:05 UTC

[GitHub] [arrow] raulcd opened a new pull request, #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

raulcd opened a new pull request, #12816:
URL: https://github.com/apache/arrow/pull/12816

   This PR fixes ARROW-16025 (Calling nonexistent method of pyarrow.orc.ORCWriter causes segfault).
   
   The segmentation fault can be reproduced with the test:
   ```python
   def test_wrong_usage_orc_writer(tempdir):
       from pyarrow import orc
   
       path = str(tempdir / 'test.orc')
       with orc.ORCWriter(path) as writer:
           with pytest.raises(AttributeError):
               writer.test()
   ```
   
   The issue was that closing `ORCFileWriter` without actually writing was trying to close a null writer (`writer_->close();`) causing the segmentation fault.


-- 
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 #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #12816:
URL: https://github.com/apache/arrow/pull/12816#issuecomment-1098615179

   Benchmark runs are scheduled for baseline = 5441c4b10c0ce44315f18b2ff6c6970ad62258de and contender = ccaef092c297b8af5d375bc542d908cda8fd415e. ccaef092c297b8af5d375bc542d908cda8fd415e 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:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/d872f5413cc04d8a857b684663b370d2...2e9d87d7fd6e47f28db724865b2cd65d/)
   [Finished :arrow_down:1.17% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/39a98094f9824f338c55296879314756...933ccb229ab440cba868d29028c0f6c2/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/eb353218214b4531ae0ad5c6e0eefe7d...e82c67e137f24b6296b68fc3359899ee/)
   [Finished :arrow_down:0.17% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c24f3f0e44f24caea1950052d42bf2f2...afd4a2f941274236a8a02be32c3d7404/)
   Buildkite builds:
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/498| `ccaef092` ec2-t3-xlarge-us-east-2>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/484| `ccaef092` test-mac-arm>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/484| `ccaef092` ursa-i9-9960x>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/494| `ccaef092` ursa-thinkcentre-m75q>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/497| `5441c4b1` ec2-t3-xlarge-us-east-2>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/483| `5441c4b1` test-mac-arm>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/483| `5441c4b1` ursa-i9-9960x>
   [Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/493| `5441c4b1` ursa-thinkcentre-m75q>
   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] raulcd commented on pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
raulcd commented on PR #12816:
URL: https://github.com/apache/arrow/pull/12816#issuecomment-1091648608

   As discussed with @jorisvandenbossche I have added a test that was causing a segmentation fault previously as seen on [ARROW-15723](https://issues.apache.org/jira/browse/ARROW-15723) which is solved as part of this 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


[GitHub] [arrow] amol- commented on pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
amol- commented on PR #12816:
URL: https://github.com/apache/arrow/pull/12816#issuecomment-1097885519

   The failed test is unrelated, merging


-- 
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] jorisvandenbossche commented on a diff in pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #12816:
URL: https://github.com/apache/arrow/pull/12816#discussion_r845062725


##########
cpp/src/arrow/adapters/orc/adapter_test.cc:
##########
@@ -406,6 +423,10 @@ TEST(TestAdapterRead, ReadIntAndStringFileMultipleStripes) {
 
 // Trivial
 
+class TestORCWriterTrivialNoWrite : public ::testing::Test {};
+TEST_F(TestORCWriterTrivialNoWrite, noWrite) {
+  TestORCWriterNoWrite(kDefaultSmallMemStreamSize / 16);

Review Comment:
   Is there a reason to not just put the test code here instead of going through the helper function? The other tests often use some helper function, but that is maybe always because it is used multiple times?



-- 
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] amol- closed pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
amol- closed pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter
URL: https://github.com/apache/arrow/pull/12816


-- 
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] raulcd commented on a diff in pull request #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #12816:
URL: https://github.com/apache/arrow/pull/12816#discussion_r845126064


##########
cpp/src/arrow/adapters/orc/adapter_test.cc:
##########
@@ -406,6 +423,10 @@ TEST(TestAdapterRead, ReadIntAndStringFileMultipleStripes) {
 
 // Trivial
 
+class TestORCWriterTrivialNoWrite : public ::testing::Test {};
+TEST_F(TestORCWriterTrivialNoWrite, noWrite) {
+  TestORCWriterNoWrite(kDefaultSmallMemStreamSize / 16);

Review Comment:
   There is no real reason. I was following the convention I saw for the other tests on `adapter_test.cc` but I have pushed bcadad3eacdf505807c02f1823932ed285e07be8 removing the helper function as I agree with it not being necessary. Thanks!
   I've also fixed the minor linting issue on the same commit.



-- 
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 #12816: ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #12816:
URL: https://github.com/apache/arrow/pull/12816#issuecomment-1090515613

   https://issues.apache.org/jira/browse/ARROW-16025


-- 
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