You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Daniel Barclay <db...@maprtech.com> on 2015/05/04 20:38:06 UTC
RecordBatchLoader.load(...) SchemaChangeException
In RecordBatchLoader, the load(...) method is declared to throw
SchemaChangeException, but it never actually throws SchemaChangeException.
It supposed to be declared to throw SchemaChangeException? (E.g., are we
reserving the "right" for load(...) to throw that, and declaring "throws
SchemaChangeException" to help make sure callers already handle it in case
load(...) later changes to actually throw it sometimes?)
Or is that "throws" a remnant that should be removed sometime?
Daniel
--
Daniel Barclay
MapR Technologies
Re: RecordBatchLoader.load(...) SchemaChangeException
Posted by Jacques Nadeau <ja...@apache.org>.
Daniel, your description is correct. Let's remove if we're not using them
anymore.
On Mon, May 4, 2015 at 1:18 PM, Daniel Barclay <db...@maprtech.com>
wrote:
> To clarify:
>
> load(...) returns a boolean value to indicate whether there was a schema
> change.
>
> Initially, I thought that throwing SchemaChangeException was an old way of
> indicating a schema change, and so thought that the throws declaration and
> various catches were obsolete.
>
> However, reportedly SchemaChangeException is or was for reporting
> /problems/ in making schema changes, not for reporting normal schema
> changes.
>
> Therefore, the code is not /necessarily /as obsolete as I initially
> thought.
>
> With that clarification, I re-submit the question:
>
>
> I wrote:
>
>> In RecordBatchLoader, the load(...) method is declared to throw
>> SchemaChangeException, but it never actually throws SchemaChangeException.
>>
>> It supposed to be declared to throw SchemaChangeException? (E.g., are we
>> reserving the "right" for load(...) to throw that, and declaring "throws
>> SchemaChangeException" to help make sure callers already handle it in case
>> load(...) later changes to actually throw it sometimes?)
>>
>> Or is that "throws" a remnant that should be removed sometime?
>>
>>
>> Daniel
>>
>
>
> --
> Daniel Barclay
> MapR Technologies
>
>
Re: RecordBatchLoader.load(...) SchemaChangeException
Posted by Daniel Barclay <db...@maprtech.com>.
To clarify:
load(...) returns a boolean value to indicate whether there was a schema change.
Initially, I thought that throwing SchemaChangeException was an old way of indicating a schema change, and so thought that the throws declaration and various catches were obsolete.
However, reportedly SchemaChangeException is or was for reporting /problems/ in making schema changes, not for reporting normal schema changes.
Therefore, the code is not /necessarily /as obsolete as I initially thought.
With that clarification, I re-submit the question:
I wrote:
> In RecordBatchLoader, the load(...) method is declared to throw
> SchemaChangeException, but it never actually throws SchemaChangeException.
>
> It supposed to be declared to throw SchemaChangeException? (E.g., are we
> reserving the "right" for load(...) to throw that, and declaring "throws
> SchemaChangeException" to help make sure callers already handle it in case
> load(...) later changes to actually throw it sometimes?)
>
> Or is that "throws" a remnant that should be removed sometime?
>
>
> Daniel
--
Daniel Barclay
MapR Technologies
Re: RecordBatchLoader.load(...) SchemaChangeException
Posted by Chris Westin <ch...@gmail.com>.
I've seen a lot of these (this and other exceptions), and they seem to be
remnants from an earlier time when things worked differently. So I just
remove them.
On Mon, May 4, 2015 at 11:59 AM, Hanifi Gunes <hg...@maprtech.com> wrote:
> We now use return value as an indicator of schema change. As Steven says,
> throws statement should be removed.
>
> On Mon, May 4, 2015 at 11:54 AM, Steven Phillips <sp...@maprtech.com>
> wrote:
>
> > I think it is most likely a remnant that should be removed.
> >
> > On Mon, May 4, 2015 at 11:38 AM, Daniel Barclay <db...@maprtech.com>
> > wrote:
> >
> > > In RecordBatchLoader, the load(...) method is declared to throw
> > > SchemaChangeException, but it never actually throws
> > SchemaChangeException.
> > >
> > > It supposed to be declared to throw SchemaChangeException? (E.g., are
> we
> > > reserving the "right" for load(...) to throw that, and declaring
> "throws
> > > SchemaChangeException" to help make sure callers already handle it in
> > case
> > > load(...) later changes to actually throw it sometimes?)
> > >
> > > Or is that "throws" a remnant that should be removed sometime?
> > >
> > >
> > > Daniel
> > > --
> > > Daniel Barclay
> > > MapR Technologies
> > >
> >
> >
> >
> > --
> > Steven Phillips
> > Software Engineer
> >
> > mapr.com
> >
>
Re: RecordBatchLoader.load(...) SchemaChangeException
Posted by Hanifi Gunes <hg...@maprtech.com>.
We now use return value as an indicator of schema change. As Steven says,
throws statement should be removed.
On Mon, May 4, 2015 at 11:54 AM, Steven Phillips <sp...@maprtech.com>
wrote:
> I think it is most likely a remnant that should be removed.
>
> On Mon, May 4, 2015 at 11:38 AM, Daniel Barclay <db...@maprtech.com>
> wrote:
>
> > In RecordBatchLoader, the load(...) method is declared to throw
> > SchemaChangeException, but it never actually throws
> SchemaChangeException.
> >
> > It supposed to be declared to throw SchemaChangeException? (E.g., are we
> > reserving the "right" for load(...) to throw that, and declaring "throws
> > SchemaChangeException" to help make sure callers already handle it in
> case
> > load(...) later changes to actually throw it sometimes?)
> >
> > Or is that "throws" a remnant that should be removed sometime?
> >
> >
> > Daniel
> > --
> > Daniel Barclay
> > MapR Technologies
> >
>
>
>
> --
> Steven Phillips
> Software Engineer
>
> mapr.com
>
Re: RecordBatchLoader.load(...) SchemaChangeException
Posted by Steven Phillips <sp...@maprtech.com>.
I think it is most likely a remnant that should be removed.
On Mon, May 4, 2015 at 11:38 AM, Daniel Barclay <db...@maprtech.com>
wrote:
> In RecordBatchLoader, the load(...) method is declared to throw
> SchemaChangeException, but it never actually throws SchemaChangeException.
>
> It supposed to be declared to throw SchemaChangeException? (E.g., are we
> reserving the "right" for load(...) to throw that, and declaring "throws
> SchemaChangeException" to help make sure callers already handle it in case
> load(...) later changes to actually throw it sometimes?)
>
> Or is that "throws" a remnant that should be removed sometime?
>
>
> Daniel
> --
> Daniel Barclay
> MapR Technologies
>
--
Steven Phillips
Software Engineer
mapr.com