You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Zhuo Peng <br...@gmail.com> on 2019/11/08 18:27:33 UTC

ConcatenateTables APIs

Hi,

https://github.com/apache/arrow/pull/5534 introduced ConcatenateTablesWithPromotion(). And there is already a ConcatenateTables() function which behaves differently (it requires the tables to have the schema). Wes raised a concern in that PR [1] that we might end up having many ConcatenateTables*() variants as there are various things that can be tweaked and he suggested to introduce a ConcatenateOptions so there is only one ConcatenateTables() function. 

While I'm onboard with that idea, I wanted to double check that there is a consensus that we should (as of today) merge ConcatenateTables() and ConcatenateTablesWithPromotion(), and have an option to do promotion or not (as in an earlier comment in the PR, @bkietz advised otherwise, but maybe at that point we didn't realize there were potentially many variants).

[1] https://github.com/apache/arrow/pull/5534#discussion_r343745573


Thanks,

Zhuo

Re: ConcatenateTables APIs

Posted by Wes McKinney <we...@gmail.com>.
I agree with introducing ConcatenateTables with an options argument
(which can have parameters added to it without disrupting public APIs
too much). It would be good to do this sooner rather than later


On Fri, Nov 15, 2019 at 12:22 AM Micah Kornfield <em...@gmail.com> wrote:
>
> This sounds like a reasonable design to me.  One question I had for
> SchemaUnificationOptions will those only be applicable to Arrow schemas or
> does it make sense to extend them for other use-cases (like DataSet APIs).
>
> Cheers,
> Micah
>
> On Fri, Nov 8, 2019 at 10:27 AM Zhuo Peng <br...@gmail.com> wrote:
>
> > Hi,
> >
> > https://github.com/apache/arrow/pull/5534 introduced
> > ConcatenateTablesWithPromotion(). And there is already a
> > ConcatenateTables() function which behaves differently (it requires the
> > tables to have the schema). Wes raised a concern in that PR [1] that we
> > might end up having many ConcatenateTables*() variants as there are various
> > things that can be tweaked and he suggested to introduce a
> > ConcatenateOptions so there is only one ConcatenateTables() function.
> >
> > While I'm onboard with that idea, I wanted to double check that there is a
> > consensus that we should (as of today) merge ConcatenateTables() and
> > ConcatenateTablesWithPromotion(), and have an option to do promotion or not
> > (as in an earlier comment in the PR, @bkietz advised otherwise, but maybe
> > at that point we didn't realize there were potentially many variants).
> >
> > [1] https://github.com/apache/arrow/pull/5534#discussion_r343745573
> >
> >
> > Thanks,
> >
> > Zhuo
> >

Re: ConcatenateTables APIs

Posted by Micah Kornfield <em...@gmail.com>.
This sounds like a reasonable design to me.  One question I had for
SchemaUnificationOptions will those only be applicable to Arrow schemas or
does it make sense to extend them for other use-cases (like DataSet APIs).

Cheers,
Micah

On Fri, Nov 8, 2019 at 10:27 AM Zhuo Peng <br...@gmail.com> wrote:

> Hi,
>
> https://github.com/apache/arrow/pull/5534 introduced
> ConcatenateTablesWithPromotion(). And there is already a
> ConcatenateTables() function which behaves differently (it requires the
> tables to have the schema). Wes raised a concern in that PR [1] that we
> might end up having many ConcatenateTables*() variants as there are various
> things that can be tweaked and he suggested to introduce a
> ConcatenateOptions so there is only one ConcatenateTables() function.
>
> While I'm onboard with that idea, I wanted to double check that there is a
> consensus that we should (as of today) merge ConcatenateTables() and
> ConcatenateTablesWithPromotion(), and have an option to do promotion or not
> (as in an earlier comment in the PR, @bkietz advised otherwise, but maybe
> at that point we didn't realize there were potentially many variants).
>
> [1] https://github.com/apache/arrow/pull/5534#discussion_r343745573
>
>
> Thanks,
>
> Zhuo
>