Okay, I've done as you suggested with PInteger, PFloat, &c and cleaned out all the funky java.lang import handling. This has brought the patch down in size significantly. I can probably reduce the patch size further still if I rename the instances, like Binary.INSTANCE to BINARY and use static imports everywhere the enum was used previously. I've also got the full test suite passing. My use of equals vs == is still inconsistent, but I'm using singletons everywhere, so this isn't causing any problems. I plan to clean that up next. I've squished the branch into a single patch and pushed to my github. Please have a look if you find a few more minutes. I guess it's time to open a JIRA now too.
https://github.com/ndimiduk/phoenix/commit/f97047b0f3e2cc7c4f60625b9eda88987156af92 On Mon, Dec 8, 2014 at 5:52 PM, James Taylor <jamestay...@apache.org> wrote: > +1 on using PInteger, PLong, etc. to disambiguate. > > +1 on limiting to purely structural changes initially. > > If it can be b/w compatible (with all tests passing), I'd vote to put > it in 4.x and master. The longer we can keep 4.1 and master in sync, > the better. > > On Mon, Dec 8, 2014 at 2:44 PM, Nick Dimiduk <ndimi...@gmail.com> wrote: > > On Thu, Dec 4, 2014 at 11:11 AM, James Taylor <jamestay...@apache.org> > > wrote: > > > >> Wow, this is fantastic, Nick. Big +1. > >> > > > > Thanks for the enthusiasm James :) > > > > You're welcome to coopt and use the type-system branch in the Apache > >> Phoenix git repo if that's helpful. > >> > > > > I'd forgotten about that one. Will keep it in mind. > > > > Any thoughts on how we can manage backward compatibility? Types are > >> identified by their ordinal position in the enum right now (that's > >> what the client typically sends to the server). If we can maintain > >> that, we might be able to pull it off. > >> > > > > I've been able to preserve the enum ordering, at least in theory. I'm > still > > working through failing tests. > > > > By breaking it up the way you've done, we should be able to get rid of > >> much of the copy/paste code that was required because we couldn't have > >> intermediate base types. For example, we can introduce a BaseArrayType > >> and move the array code their (it's more or less identical for all the > >> array sub types). The same would apply to numeric types Byte, Short, > >> Integer, and Long: we could have a BaseNumberType and remove a bunch > >> of duplicate code. > > > > > > Right. For now, I'm trying to keep the patch as limited as possible to > > structural changes. We can go back after and refactor, reduce > duplication, > > etc. > > > > Minor nit: it'd be nice if the type class names didn't conflict with > >> > > the Java built-in types so that we don't have to fully qualify them on > >> usage. > >> > > > > I've run into a couple bugs because of this already, it seems to have > made > > things fragile. OTOH, I didn't want to introduce PInteger, PLong, &c. > Maybe > > I'll go back to that, unless you have a better suggestion. > > > > It'd be great to get this in sooner rather than later, as it's going > >> to be tricky to keep your branch in sync with the Apache ones given > >> how all encompassing the change is. Any thoughts on this? > > > > > > Yes, as would I, at least with the big "bust it up" patch. Right now I'm > > working against master. Is there any reason I should be back porting it > to > > 3.x or 4.x? > > > > On Wed, Dec 3, 2014 at 1:45 PM, Nick Dimiduk <ndimi...@gmail.com> wrote: > >> > Here's my progress in the effort of breaking up the PDataType enum. > >> > > >> > https://github.com/ndimiduk/phoenix/commits/WIP-DataType > >> > > >> > On Wed, Dec 3, 2014 at 10:36 AM, Nick Dimiduk <ndimi...@gmail.com> > >> wrote: > >> > > >> >> Heya, > >> >> > >> >> I'd like to start a conversation around the idea of user-defined > types. > >> I > >> >> think this is a very powerful point of extension for a database and > will > >> >> help foster the growing community around Phoenix. It will also > >> facilitate > >> >> enhanced interoperability between Phoenix and other HBase > applications. > >> >> > >> >> I've started work on a patch to bust the PDataType enum. Rather than > a > >> >> fixed set of types, PDataType becomes an interface with the various > >> >> implementations. Probably the next step would be to extend the > grammar > >> to > >> >> support new type names and constants. After that, adding a syntax for > >> >> registering types at runtime. > >> >> > >> >> Right now this is an experiment. I'm curious if there's interest for > >> this > >> >> kind of thing in Phoenix. I draw inspiration from the extensibility > of > >> >> PostgreSQL, with a notable extension being PostGIS. As an example, > I'd > >> love > >> >> to see this feature working such that we can define a Phoenix schema > >> over > >> >> an existing OpenTSDB table. It'll take some work to get there, but I > >> think > >> >> it's worth while to help folks migrate from existing HBase schema > over > >> to > >> >> Phoenix. > >> >> > >> >> Thoughts? > >> >> > >> >> Thanks, > >> >> Nick > >> >> > >> >
