Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
MIME-Version: 1.0
In-Reply-To: <ED697E72-9DEE-47F2-BAAD-57ABBC51359B@gmail.com>
References: <CALuQ=hGAD62BEV4aSLA74vf0so_Sg8E6WZ31Y_iBz48LN_8FNQ@mail.gmail.com>
 <ED697E72-9DEE-47F2-BAAD-57ABBC51359B@gmail.com>
From: Daniel Vimont <daniel@commonvox.org>
Date: Fri, 14 Oct 2016 10:45:02 +0900
Message-ID: <CALuQ=hG3LEWqRRWw76b4rr=abu8eTN7mB0dQJQM-C0C5m0RwTA@mail.gmail.com>
Subject: Re: [DISCUSSION] Bugs in some of the OrderedBytes decode methods?
To: dev@hbase.apache.org
Content-Type: multipart/alternative; boundary=001a11407650f15be7053ec95f61
archived-at: Fri, 14 Oct 2016 01:45:27 -0000

--001a11407650f15be7053ec95f61
Content-Type: text/plain; charset=UTF-8

Agreed, Ted. But I wanted to be sure there wasn't some hidden reason for
the current "assert"-only code. The only other possibility that came to
mind is that there may be some interoperability issue(s) with external
consumers of OrderedBytes (such as Phoenix) which requires that no
exception be thrown by some of the #decode methods. Hoping that Nick D. can
perhaps provide the final word on this.


On Fri, Oct 14, 2016 at 9:29 AM, Ted Yu <yuzhihong@gmail.com> wrote:

> I think throwing exception should be the action to take.
>
> Relying on assertion is not robust.
>
> > On Oct 13, 2016, at 5:06 PM, Daniel Vimont <daniel@commonvox.org> wrote:
> >
> > I'm currently looking into the various implementations of `DataType` in
> > hbase-common, and I just wanted to get confirmation of something before I
> > open up a JIRA and fix what **appear** to be bugs in the underlying
> > OrderedBytes
> > code.
> >
> > All `DataType` implementations have their own overrides of the `#decode`
> > method. Some of these throw an appropriate exception when an invalid
> > byte-array is passed to them; for example:
> >
> > *Number bogusNumeric = OrderedNumeric.ASCENDING.decode(new
> > SimplePositionedMutableByteRange(Bytes.toBytes("xyzpdq")));*
> >
> > (This throws an IllegalArgumentException: "unexpected value in first
> byte:
> > 0x78".)
> >
> > But for other implementations, *no* validation is done; for example:
> >
> > *Short bogusShort = OrderedInt16.ASCENDING.decode(new
> > SimplePositionedMutableByteRange(Bytes.toBytes("xyzpdq")));*
> >
> > (This returns a short value of 1669, without complaint -- by ignoring the
> > first invalid "header" byte and constructing the value 1669 from the two
> > bytes that follow.)
> >
> > In those implementations which lack validation, there are "assert"
> > statements in the place where I would expect an exception to be
> explicitly
> > thrown (or, in the context of OrderedBytes, one would expect the
> > #unexpectedHeader
> > method to be invoked, which in turn throws the exception). I just wanted
> to
> > check to make sure whether (perhaps for the sake of extreme efficiency?)
> > some validations in HBase low-level processing are intentionally being
> done
> > via bypassable "assert" statements instead of the throwing of exceptions.
> >
> > Thanks!
> >
> > Dan
> >
> >
> > [image: --]
> >
> > Daniel Vimont
> > [image: https://]about.me/dvimont
> > <https://about.me/dvimont?promo=email_sig&utm_source=
> email_sig&utm_medium=external_link&utm_campaign=chrome_ext>
>

--001a11407650f15be7053ec95f61--