Skip to content

Conversation

@liyafan82
Copy link
Contributor

@liyafan82 liyafan82 commented Mar 26, 2020

After ARROW-7505 and ARROW-7935 are done, we are ready to move ArrowBuf into Arrow's package, and make it independent of Netty library.

This is part of the work for ARROW-4526, which removes netty references from ArrowBuf.

@github-actions
Copy link

Copy link
Contributor

@siddharthteotia siddharthteotia left a comment

Choose a reason for hiding this comment

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

LGTM

@siddharthteotia
Copy link
Contributor

It will be good to link the related issues in the PR description.

@siddharthteotia siddharthteotia merged commit 63308c5 into apache:master May 4, 2020
@liyafan82
Copy link
Contributor Author

It will be good to link the related issues in the PR description.

@siddharthteotia Thanks a lot for your effort. I have updated the description.

@kou
Copy link
Member

kou commented Jun 14, 2020

This breaks Spark: https://github.com/ursa-labs/crossbow/runs/769424833#step:6:13025

 [ERROR] [Error] /spark/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java:20: cannot find symbol
  symbol:   class ArrowBuf
  location: package io.netty.buffer
[ERROR] [Error] /spark/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java:461: cannot find symbol
  symbol:   class ArrowBuf
  location: class org.apache.spark.sql.vectorized.ArrowColumnVector.ArrayAccessor

Because Spark uses io.netty.buffer.ArrowBuf: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java#L20

Should we restore the io.netty.buffer.ArrowBuf name or update Spark?

@liyafan82
Copy link
Contributor Author

This breaks Spark: https://github.com/ursa-labs/crossbow/runs/769424833#step:6:13025

 [ERROR] [Error] /spark/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java:20: cannot find symbol
  symbol:   class ArrowBuf
  location: package io.netty.buffer
[ERROR] [Error] /spark/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java:461: cannot find symbol
  symbol:   class ArrowBuf
  location: class org.apache.spark.sql.vectorized.ArrowColumnVector.ArrayAccessor

Because Spark uses io.netty.buffer.ArrowBuf: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java#L20

Should we restore the io.netty.buffer.ArrowBuf name or update Spark?

Hi @kou, thanks a lot for reporting the problem.
I'd prefer updating Spark, as this PR represents one of the steps towards moving netty related code into a separate module.
We have retried to keep two implementations of ArrowBuf, and make one as deprecated. However, that would cause some other problems, so we chose to directly move ArrowBuf to another package.

@kou
Copy link
Member

kou commented Jun 15, 2020

@liyafan82 OK. Could you open an JIRA issue for Spark to notify this to Spark developers? https://issues.apache.org/jira/browse/SPARK
FYI: https://spark.apache.org/contributing.html

@kiszk Could you support updating Spark for this change?

@kiszk
Copy link
Member

kiszk commented Jun 16, 2020

I see. Since ArrowBuf package name is changed, IMHO, a PR for this change in Spark should include upgrading Arrow from 0.15.1 to 0.17.1.

Since Spark 3.0 branch is already cut, upgrading looks easier than in early 2020. I will support it.

@liyafan82
Copy link
Contributor Author

@liyafan82 OK. Could you open an JIRA issue for Spark to notify this to Spark developers? https://issues.apache.org/jira/browse/SPARK
FYI: https://spark.apache.org/contributing.html

@kiszk Could you support updating Spark for this change?

Sure. I have opened https://issues.apache.org/jira/browse/SPARK-31998 to track it.

@kou
Copy link
Member

kou commented Jun 16, 2020

Thanks!

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.

5 participants