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/06/02 08:26:01 UTC

[GitHub] [arrow] kou opened a new pull request, #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   Our bundled Thrift is 0.16.0. Users can use other version but will not
   use 0.13 or earlier.
   
   This change also cleans FindThrift.cmake up.
   
   This change also fixes build errors with -DARROW_HIVESERVER2=ON. We
   may be able to remove cpp/src/arrow/dbi/hiveserver2/. It seems that
   nobody uses 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] pitrou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   There's some compatibility code lying around (look for PARQUET_THRIFT_USE_BOOST and FORCE_BOOST_SMART_PTR), we should probably remove it as well.


-- 
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 #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

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


-- 
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] kou commented on a diff in pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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


##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1011,7 +983,7 @@ if(ARROW_BUILD_INTEGRATION
   set(ARROW_USE_BOOST TRUE)
   set(ARROW_BOOST_REQUIRE_LIBRARY TRUE)
 elseif(ARROW_GANDIVA
-       OR (ARROW_WITH_THRIFT AND THRIFT_REQUIRES_BOOST)
+       OR ARROW_WITH_THRIFT

Review Comment:
   Yes. The condition was wrong. We needs Boost to use Thrift.
   
   We need Thrift only when we use Parquet or `arrow/dbi/hiveserver2/`.
   Parquet uses `thrift/transport/TBufferTransports.h`: https://github.com/apache/arrow/blob/master/cpp/src/parquet/thrift_internal.h#L43
   `thrift/transport/TBufferTransports.h` uses Boost: https://github.com/apache/thrift/blob/master/lib/cpp/src/thrift/transport/TBufferTransports.h#L26



-- 
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] pitrou merged pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

Posted by GitBox <gi...@apache.org>.
pitrou merged PR #13292:
URL: https://github.com/apache/arrow/pull/13292


-- 
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] nealrichardson commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   > I've fixed R failures but new R failures are happen. The new failures are also happen on master: https://github.com/apache/arrow/runs/6747296145?check_suite_focus=true
   > 
   > It seems that duckdb is updated to 0.3.4 from 0.3.2. Cc: @nealrichardson
   
   Yeah I noticed that too. We'll figure out something today.


-- 
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] kou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   I've fixed R failures but new R failures are happen. The new failures are also happen on master: https://github.com/apache/arrow/runs/6747296145?check_suite_focus=true
   
   It seems that duckdb is updated to 0.3.4 from 0.3.2.
   Cc: @nealrichardson


-- 
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] kou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   > There's some compatibility code lying around (look for PARQUET_THRIFT_USE_BOOST and FORCE_BOOST_SMART_PTR), we should probably remove it as well.
   
   Good catch!
   I've removed them because they are for Thrift < 0.11 that isn't supported.


-- 
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] kou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   Thanks!
   
   This is ready to merge.


-- 
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] kou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   I've started a discussion for removing `cpp/src/arrow/dbi/hiveserver2/`:
   [C++] Can we remove cpp/src/arrow/dbi/hiveserver2? https://lists.apache.org/thread/70qv1q9krx7ztk35tzxq8jp11vq5b5zt
   


-- 
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] pitrou commented on a diff in pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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


##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1011,7 +983,7 @@ if(ARROW_BUILD_INTEGRATION
   set(ARROW_USE_BOOST TRUE)
   set(ARROW_BOOST_REQUIRE_LIBRARY TRUE)
 elseif(ARROW_GANDIVA
-       OR (ARROW_WITH_THRIFT AND THRIFT_REQUIRES_BOOST)
+       OR ARROW_WITH_THRIFT

Review Comment:
   So boost is always required by Thrift nevertheless?



-- 
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 #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
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] pitrou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   > We
   > may be able to remove cpp/src/arrow/dbi/hiveserver2/. It seems that
   > nobody uses it.
   
   +1


-- 
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] nealrichardson commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   https://github.com/apache/arrow/pull/13323 is merged; rebase should make this go away


-- 
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] pitrou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   The R CI failures look related.


-- 
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] kou commented on pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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

   > I've started a discussion for removing `cpp/src/arrow/dbi/hiveserver2/`:
   > [C++] Can we remove cpp/src/arrow/dbi/hiveserver2? https://lists.apache.org/thread/70qv1q9krx7ztk35tzxq8jp11vq5b5zt
   
   I'll remove `cpp/src/arrow/dbi/hiveserver2/` in a follow-up pull request once we get a consensus of 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] pitrou commented on a diff in pull request #13292: ARROW-16721: [C++] Drop support for bundled Thrift < 0.13

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


##########
cpp/src/arrow/dbi/hiveserver2/CMakeLists.txt:
##########
@@ -31,29 +31,32 @@ set(ARROW_HIVESERVER2_SRCS
     types.cc
     util.cc)
 
+set(HIVESERVER2_THRIFT_SRC_DIR "${ARROW_BINARY_DIR}/src/arrow/dbi/hiveserver2")

Review Comment:
   I think our hiveserver adapter doesn't even build currently, do we want to bother fixing 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