You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by Gabor Szadovszky <ga...@apache.org> on 2019/02/19 16:44:21 UTC
Reverting the merge blocks command feature
Hi All,
During working on a fix I've discovered that the recently added (since
1.10.0) feature PARQUET-1381 is not properly implemented and causes some
unit test failures with my independent fix. After a more deep investigation
I think the design of this feature is conceptionally incompatible with our
current API. See the comments of the jira for more details.
Because I don't feel this feature very important for the current release I
think the most easier and painless solution is to revert it. Created the PR
#620 for it.
We usually don't do reverts especially for commits that are sitting in
master for a while. I would like to ask your opinions about it. Also feel
free to comment in the jira (about the design/implementation of the
feature) or the PR (about the revert).
Thanks a lot,
Gabor
Re: Reverting the merge blocks command feature
Posted by Gabor Szadovszky <ga...@cloudera.com.INVALID>.
Yes, it is related to PARQUET-1414. During the fix of PARQUET-1531 I've
added an exception to be thrown in the case when an empty page would be
written. Because of that I've discovered that the unit tests of this merge
features throw this exception so I started to investigate the
implementation of the feature. For the details of this investigation see
the PARQUET-1381.
On Thu, Feb 21, 2019 at 6:43 PM Ryan Blue <rb...@netflix.com.invalid> wrote:
> Was the motivation for this the bug that was found with PARQUET-1414? How
> did we catch this?
>
> On Thu, Feb 21, 2019 at 4:56 AM Gabor Szadovszky <ga...@apache.org> wrote:
>
> > HI All,
> >
> > I'm planning to push the revert tomorrow if there are no objections.
> >
> > Cheers,
> > Gabor
> >
> > On Tue, Feb 19, 2019 at 6:02 PM Gabor Szadovszky <ga...@apache.org>
> wrote:
> >
> > > Sorry, wrong PR. So, see PARQUET-1381
> > > <https://issues.apache.org/jira/browse/PARQUET-1381> and PR #621
> > > <https://github.com/apache/parquet-mr/pull/621>.
> > >
> > > On Tue, Feb 19, 2019 at 5:44 PM Gabor Szadovszky <ga...@apache.org>
> > wrote:
> > >
> > >> Hi All,
> > >>
> > >> During working on a fix I've discovered that the recently added (since
> > >> 1.10.0) feature PARQUET-1381 is not properly implemented and causes
> some
> > >> unit test failures with my independent fix. After a more deep
> > investigation
> > >> I think the design of this feature is conceptionally incompatible with
> > our
> > >> current API. See the comments of the jira for more details.
> > >>
> > >> Because I don't feel this feature very important for the current
> release
> > >> I think the most easier and painless solution is to revert it. Created
> > the
> > >> PR #620 for it.
> > >>
> > >> We usually don't do reverts especially for commits that are sitting in
> > >> master for a while. I would like to ask your opinions about it. Also
> > feel
> > >> free to comment in the jira (about the design/implementation of the
> > >> feature) or the PR (about the revert).
> > >>
> > >> Thanks a lot,
> > >> Gabor
> > >>
> > >
> >
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
>
Re: Reverting the merge blocks command feature
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
Was the motivation for this the bug that was found with PARQUET-1414? How
did we catch this?
On Thu, Feb 21, 2019 at 4:56 AM Gabor Szadovszky <ga...@apache.org> wrote:
> HI All,
>
> I'm planning to push the revert tomorrow if there are no objections.
>
> Cheers,
> Gabor
>
> On Tue, Feb 19, 2019 at 6:02 PM Gabor Szadovszky <ga...@apache.org> wrote:
>
> > Sorry, wrong PR. So, see PARQUET-1381
> > <https://issues.apache.org/jira/browse/PARQUET-1381> and PR #621
> > <https://github.com/apache/parquet-mr/pull/621>.
> >
> > On Tue, Feb 19, 2019 at 5:44 PM Gabor Szadovszky <ga...@apache.org>
> wrote:
> >
> >> Hi All,
> >>
> >> During working on a fix I've discovered that the recently added (since
> >> 1.10.0) feature PARQUET-1381 is not properly implemented and causes some
> >> unit test failures with my independent fix. After a more deep
> investigation
> >> I think the design of this feature is conceptionally incompatible with
> our
> >> current API. See the comments of the jira for more details.
> >>
> >> Because I don't feel this feature very important for the current release
> >> I think the most easier and painless solution is to revert it. Created
> the
> >> PR #620 for it.
> >>
> >> We usually don't do reverts especially for commits that are sitting in
> >> master for a while. I would like to ask your opinions about it. Also
> feel
> >> free to comment in the jira (about the design/implementation of the
> >> feature) or the PR (about the revert).
> >>
> >> Thanks a lot,
> >> Gabor
> >>
> >
>
--
Ryan Blue
Software Engineer
Netflix
Re: Reverting the merge blocks command feature
Posted by Gabor Szadovszky <ga...@apache.org>.
HI All,
I'm planning to push the revert tomorrow if there are no objections.
Cheers,
Gabor
On Tue, Feb 19, 2019 at 6:02 PM Gabor Szadovszky <ga...@apache.org> wrote:
> Sorry, wrong PR. So, see PARQUET-1381
> <https://issues.apache.org/jira/browse/PARQUET-1381> and PR #621
> <https://github.com/apache/parquet-mr/pull/621>.
>
> On Tue, Feb 19, 2019 at 5:44 PM Gabor Szadovszky <ga...@apache.org> wrote:
>
>> Hi All,
>>
>> During working on a fix I've discovered that the recently added (since
>> 1.10.0) feature PARQUET-1381 is not properly implemented and causes some
>> unit test failures with my independent fix. After a more deep investigation
>> I think the design of this feature is conceptionally incompatible with our
>> current API. See the comments of the jira for more details.
>>
>> Because I don't feel this feature very important for the current release
>> I think the most easier and painless solution is to revert it. Created the
>> PR #620 for it.
>>
>> We usually don't do reverts especially for commits that are sitting in
>> master for a while. I would like to ask your opinions about it. Also feel
>> free to comment in the jira (about the design/implementation of the
>> feature) or the PR (about the revert).
>>
>> Thanks a lot,
>> Gabor
>>
>
Re: Reverting the merge blocks command feature
Posted by Gabor Szadovszky <ga...@apache.org>.
Sorry, wrong PR. So, see PARQUET-1381
<https://issues.apache.org/jira/browse/PARQUET-1381> and PR #621
<https://github.com/apache/parquet-mr/pull/621>.
On Tue, Feb 19, 2019 at 5:44 PM Gabor Szadovszky <ga...@apache.org> wrote:
> Hi All,
>
> During working on a fix I've discovered that the recently added (since
> 1.10.0) feature PARQUET-1381 is not properly implemented and causes some
> unit test failures with my independent fix. After a more deep investigation
> I think the design of this feature is conceptionally incompatible with our
> current API. See the comments of the jira for more details.
>
> Because I don't feel this feature very important for the current release I
> think the most easier and painless solution is to revert it. Created the PR
> #620 for it.
>
> We usually don't do reverts especially for commits that are sitting in
> master for a while. I would like to ask your opinions about it. Also feel
> free to comment in the jira (about the design/implementation of the
> feature) or the PR (about the revert).
>
> Thanks a lot,
> Gabor
>