You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Hongze Zhang <no...@126.com> on 2021/08/04 02:43:32 UTC
Review request for Dataset Java API PRs
Hi,
I have some PRs that were to improve Dataset API's Java implementation
have not been reviewing for months. Could someone help me to review
them? Thanks in advance.
1. https://github.com/apache/arrow/pull/10201
ARROW-11776: [Java][Dataset] Support writing to files within dataset
scanner via JNI
2. https://github.com/apache/arrow/pull/10333
ARROW-12607: [Website] Doc section for Dataset Java bindings
3. https://github.com/apache/arrow/pull/10114
ARROW-12480: [Java][Dataset] FileSystemDataset: Support reading from a
directory
4.https://github.com/apache/arrow/pull/10652
ARROW-13257: [Java][Dataset] Allow passing empty columns for projection
One of the most critical changes among the PRs is to add write support
to Java API (The first in the list). This also includes some work that
builds a common way to share Arrow data between C++ and Java over JNI.
Also this work was pretty close to the proposal in ARROW-7272[1].
Other PRs are minor improvements like the the second one to create Java
Dataset doc page on Arrow website. It also received some review
comments already.
Thanks,
Hongze
[1] https://issues.apache.org/jira/browse/ARROW-7272
Re: Review request for Dataset Java API PRs
Posted by Hongze Zhang <no...@126.com>.
Hello guys, sorry I have to to request for review here again since
progress didn't seem to be made yet :(. These PRs are important to
Dataset Java implementation as the first version of it was too basic to
serve advanced use cases. If reviewing of the big write support PR[1]
sounds to be a bit of complicated I have split the JNI-data-swapping
part to another smaller PR[2] with its own test cases.
Other PRs[3][4][5] all include minor changes so should be
straightforward to review I think. In my team we have been targeting to
enrich this part of code to make it more production-ready for being
used in JVM-based data processing systems like Apache Spark etc. The
codes have been working well and stable enough in many of our end-to-
end stress tests and benchmarks so I am pretty confident that they are
in good status to go to the next move. And some of out future
improvements will happen to be based on these changes so having a up-
to-date base will simplify further development work a lot. So if anyone
have the privilege wants to help review and merge them that would be
very much appreciated. :)
Thanks,
Hongze
[1] https://github.com/apache/arrow/pull/10201
[2] https://github.com/apache/arrow/pull/10883
[3] https://github.com/apache/arrow/pull/10333
[4] https://github.com/apache/arrow/pull/10114
[5] https://github.com/apache/arrow/pull/10652
On Thu, 2021-08-05 at 18:27 +0800, Hongze Zhang wrote:
> Thanks everyone for the quick response! By the way I might raise this
> review request a little bit late because I was working on some other
> projects in the last few months either. Now I just have some time to
> push this forward. :)
>
>
> About ARROW-11776:
>
> On Wed, 2021-08-04 at 08:45 -0700, Micah Kornfield wrote:
> > One thing that can also help is if there is a way to divide any of
> > the PRs into smaller standalone components it would likely help get
> > them merged sooner
>
> Yes. I used to refactor PR#10201 to have both fixes of ARROW-7272 (JNI
> bridge of record batch) and of ARROW-11776 (Dataset write) in it and in
> two different commits. Today I just make another individual PR for
> ARROW-7272: https://github.com/apache/arrow/pull/10883 which I think
> can be reviewed first. Hopefully it would ease the whole reviewing
> process. :)
>
>
> And regarding C data interface, I think I will be able to help
> reviewing Roee's work which I think is already in a pretty good shape
> now (maybe after a PR submitted). And once the codes get merged it
> should be easy for Dataset to migrate from current data sharing
> solution to the ABI cause I have extracted a utility layer for data
> sharing in ARROW-7272's PR.
>
> Thanks,
> Hongze
>
Re: Review request for Dataset Java API PRs
Posted by Hongze Zhang <no...@126.com>.
Thanks everyone for the quick response! By the way I might raise this
review request a little bit late because I was working on some other
projects in the last few months either. Now I just have some time to
push this forward. :)
About ARROW-11776:
On Wed, 2021-08-04 at 08:45 -0700, Micah Kornfield wrote:
> One thing that can also help is if there is a way to divide any of
> the PRs into smaller standalone components it would likely help get
> them merged sooner
Yes. I used to refactor PR#10201 to have both fixes of ARROW-7272 (JNI
bridge of record batch) and of ARROW-11776 (Dataset write) in it and in
two different commits. Today I just make another individual PR for
ARROW-7272: https://github.com/apache/arrow/pull/10883 which I think
can be reviewed first. Hopefully it would ease the whole reviewing
process. :)
And regarding C data interface, I think I will be able to help
reviewing Roee's work which I think is already in a pretty good shape
now (maybe after a PR submitted). And once the codes get merged it
should be easy for Dataset to migrate from current data sharing
solution to the ABI cause I have extracted a utility layer for data
sharing in ARROW-7272's PR.
Thanks,
Hongze
Re: Review request for Dataset Java API PRs
Posted by Paul Whalen <pg...@gmail.com>.
I would love to see a component or some standardization around Java using the C Data Interface. I’ve been prototyping JNI bindings for DataFusion in the last week or so with some success, and was getting ready to ask where/how such a thing might fit in. I’ll be sure to watch that JIRA.
(I also prototyped with a Panama EA build, but obviously we’re a long way from using that)
Paul
> On Aug 4, 2021, at 11:11 AM, Antoine Pitrou <an...@python.org> wrote:
>
>
> I don't know about the rest of these tasks, but sharing data between Arrow Java and C++ should definitely use the C data interface.
>
> It seems there's work in progress here, feel free to collaborate:
> https://issues.apache.org/jira/browse/ARROW-12965
>
> Regards
>
> Antoine.
>
>
>> Le 04/08/2021 à 17:45, Micah Kornfield a écrit :
>> Hi Hongze,
>> Sorry I started taking a look at these a while ago, but my focus has been
>> elsewhere with the time I have available to contribute to the project. One
>> thing that can also help is if there is a way to divide any of the PRs into
>> smaller standalone components it would likely help get them merged sooner
>> (I seem to recall at least one PR redid both how memory management was
>> working between C++ and Java as well as adding more functionality for
>> datasets, apologies if I am misremembering).
>> If other people have time to review that would be great.
>> Thanks,
>> Micah
>>> On Wed, Aug 4, 2021 at 6:11 AM Wes McKinney <we...@gmail.com> wrote:
>>> hi Hongze — I am not sure who will be able to review these, but in the
>>> future feel free to raise your Java PRs on the mailing list even
>>> sooner, no need to wait for more than a month. There are far fewer
>>> active Java developers vs. C++ or Rust, so it can help to get people's
>>> attention on your work.
>>>
>>> - Wes
>>>
>>> On Tue, Aug 3, 2021 at 9:44 PM Hongze Zhang <no...@126.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I have some PRs that were to improve Dataset API's Java implementation
>>>> have not been reviewing for months. Could someone help me to review
>>>> them? Thanks in advance.
>>>>
>>>> 1. https://github.com/apache/arrow/pull/10201
>>>> ARROW-11776: [Java][Dataset] Support writing to files within dataset
>>>> scanner via JNI
>>>> 2. https://github.com/apache/arrow/pull/10333
>>>> ARROW-12607: [Website] Doc section for Dataset Java bindings
>>>> 3. https://github.com/apache/arrow/pull/10114
>>>> ARROW-12480: [Java][Dataset] FileSystemDataset: Support reading from a
>>>> directory
>>>> 4.https://github.com/apache/arrow/pull/10652
>>>> ARROW-13257: [Java][Dataset] Allow passing empty columns for projection
>>>>
>>>> One of the most critical changes among the PRs is to add write support
>>>> to Java API (The first in the list). This also includes some work that
>>>> builds a common way to share Arrow data between C++ and Java over JNI.
>>>> Also this work was pretty close to the proposal in ARROW-7272[1].
>>>>
>>>> Other PRs are minor improvements like the the second one to create Java
>>>> Dataset doc page on Arrow website. It also received some review
>>>> comments already.
>>>>
>>>> Thanks,
>>>> Hongze
>>>>
>>>> [1] https://issues.apache.org/jira/browse/ARROW-7272
>>>>
>>>
Re: Review request for Dataset Java API PRs
Posted by Antoine Pitrou <an...@python.org>.
I don't know about the rest of these tasks, but sharing data between
Arrow Java and C++ should definitely use the C data interface.
It seems there's work in progress here, feel free to collaborate:
https://issues.apache.org/jira/browse/ARROW-12965
Regards
Antoine.
Le 04/08/2021 à 17:45, Micah Kornfield a écrit :
> Hi Hongze,
> Sorry I started taking a look at these a while ago, but my focus has been
> elsewhere with the time I have available to contribute to the project. One
> thing that can also help is if there is a way to divide any of the PRs into
> smaller standalone components it would likely help get them merged sooner
> (I seem to recall at least one PR redid both how memory management was
> working between C++ and Java as well as adding more functionality for
> datasets, apologies if I am misremembering).
>
> If other people have time to review that would be great.
>
> Thanks,
> Micah
>
> On Wed, Aug 4, 2021 at 6:11 AM Wes McKinney <we...@gmail.com> wrote:
>
>> hi Hongze — I am not sure who will be able to review these, but in the
>> future feel free to raise your Java PRs on the mailing list even
>> sooner, no need to wait for more than a month. There are far fewer
>> active Java developers vs. C++ or Rust, so it can help to get people's
>> attention on your work.
>>
>> - Wes
>>
>> On Tue, Aug 3, 2021 at 9:44 PM Hongze Zhang <no...@126.com> wrote:
>>>
>>> Hi,
>>>
>>> I have some PRs that were to improve Dataset API's Java implementation
>>> have not been reviewing for months. Could someone help me to review
>>> them? Thanks in advance.
>>>
>>> 1. https://github.com/apache/arrow/pull/10201
>>> ARROW-11776: [Java][Dataset] Support writing to files within dataset
>>> scanner via JNI
>>> 2. https://github.com/apache/arrow/pull/10333
>>> ARROW-12607: [Website] Doc section for Dataset Java bindings
>>> 3. https://github.com/apache/arrow/pull/10114
>>> ARROW-12480: [Java][Dataset] FileSystemDataset: Support reading from a
>>> directory
>>> 4.https://github.com/apache/arrow/pull/10652
>>> ARROW-13257: [Java][Dataset] Allow passing empty columns for projection
>>>
>>> One of the most critical changes among the PRs is to add write support
>>> to Java API (The first in the list). This also includes some work that
>>> builds a common way to share Arrow data between C++ and Java over JNI.
>>> Also this work was pretty close to the proposal in ARROW-7272[1].
>>>
>>> Other PRs are minor improvements like the the second one to create Java
>>> Dataset doc page on Arrow website. It also received some review
>>> comments already.
>>>
>>> Thanks,
>>> Hongze
>>>
>>> [1] https://issues.apache.org/jira/browse/ARROW-7272
>>>
>>
>
Re: Review request for Dataset Java API PRs
Posted by Micah Kornfield <em...@gmail.com>.
Hi Hongze,
Sorry I started taking a look at these a while ago, but my focus has been
elsewhere with the time I have available to contribute to the project. One
thing that can also help is if there is a way to divide any of the PRs into
smaller standalone components it would likely help get them merged sooner
(I seem to recall at least one PR redid both how memory management was
working between C++ and Java as well as adding more functionality for
datasets, apologies if I am misremembering).
If other people have time to review that would be great.
Thanks,
Micah
On Wed, Aug 4, 2021 at 6:11 AM Wes McKinney <we...@gmail.com> wrote:
> hi Hongze — I am not sure who will be able to review these, but in the
> future feel free to raise your Java PRs on the mailing list even
> sooner, no need to wait for more than a month. There are far fewer
> active Java developers vs. C++ or Rust, so it can help to get people's
> attention on your work.
>
> - Wes
>
> On Tue, Aug 3, 2021 at 9:44 PM Hongze Zhang <no...@126.com> wrote:
> >
> > Hi,
> >
> > I have some PRs that were to improve Dataset API's Java implementation
> > have not been reviewing for months. Could someone help me to review
> > them? Thanks in advance.
> >
> > 1. https://github.com/apache/arrow/pull/10201
> > ARROW-11776: [Java][Dataset] Support writing to files within dataset
> > scanner via JNI
> > 2. https://github.com/apache/arrow/pull/10333
> > ARROW-12607: [Website] Doc section for Dataset Java bindings
> > 3. https://github.com/apache/arrow/pull/10114
> > ARROW-12480: [Java][Dataset] FileSystemDataset: Support reading from a
> > directory
> > 4.https://github.com/apache/arrow/pull/10652
> > ARROW-13257: [Java][Dataset] Allow passing empty columns for projection
> >
> > One of the most critical changes among the PRs is to add write support
> > to Java API (The first in the list). This also includes some work that
> > builds a common way to share Arrow data between C++ and Java over JNI.
> > Also this work was pretty close to the proposal in ARROW-7272[1].
> >
> > Other PRs are minor improvements like the the second one to create Java
> > Dataset doc page on Arrow website. It also received some review
> > comments already.
> >
> > Thanks,
> > Hongze
> >
> > [1] https://issues.apache.org/jira/browse/ARROW-7272
> >
>
Re: Review request for Dataset Java API PRs
Posted by Wes McKinney <we...@gmail.com>.
hi Hongze — I am not sure who will be able to review these, but in the
future feel free to raise your Java PRs on the mailing list even
sooner, no need to wait for more than a month. There are far fewer
active Java developers vs. C++ or Rust, so it can help to get people's
attention on your work.
- Wes
On Tue, Aug 3, 2021 at 9:44 PM Hongze Zhang <no...@126.com> wrote:
>
> Hi,
>
> I have some PRs that were to improve Dataset API's Java implementation
> have not been reviewing for months. Could someone help me to review
> them? Thanks in advance.
>
> 1. https://github.com/apache/arrow/pull/10201
> ARROW-11776: [Java][Dataset] Support writing to files within dataset
> scanner via JNI
> 2. https://github.com/apache/arrow/pull/10333
> ARROW-12607: [Website] Doc section for Dataset Java bindings
> 3. https://github.com/apache/arrow/pull/10114
> ARROW-12480: [Java][Dataset] FileSystemDataset: Support reading from a
> directory
> 4.https://github.com/apache/arrow/pull/10652
> ARROW-13257: [Java][Dataset] Allow passing empty columns for projection
>
> One of the most critical changes among the PRs is to add write support
> to Java API (The first in the list). This also includes some work that
> builds a common way to share Arrow data between C++ and Java over JNI.
> Also this work was pretty close to the proposal in ARROW-7272[1].
>
> Other PRs are minor improvements like the the second one to create Java
> Dataset doc page on Arrow website. It also received some review
> comments already.
>
> Thanks,
> Hongze
>
> [1] https://issues.apache.org/jira/browse/ARROW-7272
>