Skip to content

HIVE-28904: Remove Arrow from Hive #5772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abstractdog
Copy link
Contributor

@abstractdog abstractdog commented Apr 14, 2025

What changes were proposed in this pull request?

To remove Arrow dependency and usage from hive.

Why are the changes needed?

Maintenance burden, and it's not used anymore.
https://lists.apache.org/thread/j1hclgwqytbhdbbbw12sy1nrpxvbq1c1

Does this PR introduce any user-facing change?

No, unless the user still utilizes a long-time abandoned hive warehouse connector reader mode.

Is the change a dependency upgrade?

No.

How was this patch tested?

Precommit testing.

Copy link
Contributor

@okumin okumin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can justify the use case in LLAP. I reviewed only the correctness of these changes, assuming we should discontinue the feature.

Also, this PR will remove some flaky tests. Please void the following tickets when we merge this PR.

@@ -219,26 +191,8 @@ public RecordReader<NullWritable, V> getRecordReader(InputSplit split, JobConf j

LOG.info("Registered id: " + fragmentId);

@SuppressWarnings("rawtypes")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

@@ -198,16 +198,9 @@ private void registerReader(ChannelHandlerContext ctx, String id, byte[] tokenBy
LOG.debug("registering socket for: " + id);
int maxPendingWrites = HiveConf.getIntVar(conf,
HiveConf.ConfVars.LLAP_DAEMON_OUTPUT_SERVICE_MAX_PENDING_WRITES);
boolean useArrow = HiveConf.getBoolVar(conf, HiveConf.ConfVars.LLAP_OUTPUT_FORMAT_ARROW);
@SuppressWarnings("rawtypes")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

@abstractdog
Copy link
Contributor Author

thanks @okumin for the review!
added the SuppressWarning annotation back, the related sonarqube issues have disappeared
as your +1 is binding, I'm about to merge this soon, let me wait another 12-24h for anyone else to chime in, just in case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants