No argument that naming should set expectations of immutability if that's what should be conveyed, but Guava types (or Guava anything) is a means to an end that can inflict significant pain on downstreamers.
On Tue, Sep 6, 2016 at 11:59 AM, Julian Hyde <jh...@apache.org> wrote: > Calcite’s API has a large surface area. The API consists not just of > method calls, but also data objects. For example, the Project class [1] > represents a project node in a relational algebra expression. Its main > field is “public final ImmutableList<RexNode> exps”. It is very important > that everyone, especially the client, understands that that list is > immutable. When you create a Project, you do not need to make a defensive > copy of the list because no one is able to modify it. > > Imagine the mayhem if java.lang.String was mutable. As an API designer you > would have to spell out whether the caller or the provider is allowed to > change the string, and at what time. You would worry about thread safety, > if the string has been shared with another thread. Well, I believe that > Guava immutable collections prevent the same kinds of mayhem. I would call > that good API design. > > The immutable collections and functions are in every Guava version, so we > really don’t care which Guava version we use, as long as it is not shaded. > > Julian > > [1] https://calcite.apache.org/apidocs/org/apache/calcite/ > rel/core/Project.html <https://calcite.apache.org/ > apidocs/org/apache/calcite/rel/core/Project.html> > > > On Sep 3, 2016, at 6:02 PM, Andrew Purtell <andrew.purt...@gmail.com> > wrote: > > > > I wouldn't call embedding Guava types in a public API either a service > for users nor good API design, given the pain I've personally seen it > inflict on multiple projects given Google's uncaring nature on cross > version compatibility. > > > >> On Sep 3, 2016, at 5:35 PM, Jacques Nadeau <jacq...@apache.org> wrote: > >> > >> Do you have a sense of how often we expose these? > >> > >> One random thought, shade Guava and continue to expose the shaded.guava > >> classes in public APIs. > >> > >> People could choose to use the unshaded or shaded. > >> > >>> On Sat, Sep 3, 2016 at 11:26 AM, Julian Hyde <jh...@apache.org> wrote: > >>> > >>> I'm not keen on shading Guava, because I want to include some of > >>> Guava's classes in Calcite's public API: for example ImmutableList and > >>> Function. Using these classes in APIs makes better APIs. They should > >>> be in the JDK, but sadly they're not, so we use Guava. > >>> > >>> Calcite's policy has been to support a wide range of Guava versions > >>> but to drop support for really old versions. We can use features in > >>> newer versions via reflection, as long as we don't introduce a link > >>> dependency (i.e. we call via reflection) and we can provide fallback > >>> for older versions. All of this is identical to our policy for JDKs, > >>> really. > >>> > >>> All we need is that our dependencies move off the really old versions > >>> in a timely fashion. > >>> > >>> Julian > >>> > >>> > >>> On Sat, Sep 3, 2016 at 10:20 AM, Andrew Purtell > >>> <andrew.purt...@gmail.com> wrote: > >>>> Use hbase-shaded-client as Maven dep (1.1 and up) > >>>> > >>>>> On Sep 3, 2016, at 10:12 AM, James Taylor <jamestay...@apache.org> > >>> wrote: > >>>>> > >>>>> Does shading of protobuf on the HBase client work (or is that > dependent > >>> on > >>>>> that brave work Stack is doing)? > >>>>> > >>>>> On Sat, Sep 3, 2016 at 10:10 AM, Andrew Purtell < > >>> andrew.purt...@gmail.com> > >>>>> wrote: > >>>>> > >>>>>> James - When Stack is finished coprocessors will work with shaded > >>>>>> protobuf. Not yet. > >>>>>> > >>>>>>>> On Sep 3, 2016, at 10:07 AM, James Taylor <jamestay...@apache.org > > > >>>>>>> wrote: > >>>>>>> > >>>>>>> Also agree - shading of guava & protobuf would be super valuable. > >>> Phoenix > >>>>>>> ended up not supporting shading of protobuf because of difficulties > >>>>>> getting > >>>>>>> it to work (maybe because HBase dependency?). I think we support > >>> shading > >>>>>> of > >>>>>>> Guava, though. Is that correct, Sergey? > >>>>>>> > >>>>>>>> On Sat, Sep 3, 2016 at 10:02 AM, Jacques Nadeau < > jacq...@apache.org> > >>>>>> wrote: > >>>>>>>> > >>>>>>>> +1 on shading guava/protobuf. > >>>>>>>> > >>>>>>>> On Sat, Sep 3, 2016 at 9:48 AM, Andrew Purtell < > >>>>>> andrew.purt...@gmail.com> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Since Calcite should become a widely used library (smile) I > think it > >>>>>>>> would > >>>>>>>>> be prudent to shade Guava and protobuf if Calcite depends on > them. > >>> Then > >>>>>>>> you > >>>>>>>>> will play very nicely indeed on the classpath no matter what > >>> versions > >>>>>> are > >>>>>>>>> required by calling code. > >>>>>>>>> > >>>>>>>>> Jacques - Good lord. Let me see about shading HBase use of > Guava, or > >>>>>>>>> eliminating it. Unfortunately that will be no help in the short > >>> term. > >>>>>>>>> Related, our Stack is wrestling with shading protobuf already, > and > >>> is > >>>>>>>> neck > >>>>>>>>> deep in the Swamp of Classloading at the moment. > >>>>>>>>> > >>>>>>>>>> On Sep 3, 2016, at 9:06 AM, Jacques Nadeau <jacq...@apache.org> > >>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> It isn't a real solution but in Drill we solved the HBase > >>>>>>>> incompatibility > >>>>>>>>>> issue on the server side (for tests only) by patching Guava 18 > to > >>>>>> allow > >>>>>>>>> the > >>>>>>>>>> HBase Guava calls that are missing. They are really quite > trivial > >>> and > >>>>>>>>>> support Andrew's arguments that Guava is the devil... > >>>>>>>>>> > >>>>>>>>>> https://github.com/apache/drill/blob/master/exec/java- > >>>>>>>>> exec/src/main/java/org/apache/drill/exec/util/GuavaPatcher.java > >>>>>>>>>> > >>>>>>>>>> On Sat, Sep 3, 2016 at 8:16 AM, Andrew Purtell < > >>>>>>>> andrew.purt...@gmail.com > >>>>>>>>>> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> While that seems very unfriendly of them, the main issue is > Guava > >>> is > >>>>>>>> the > >>>>>>>>>>> devil (and protobuf is a minor demon). Would shading be an > option? > >>>>>>>>>>> > >>>>>>>>>>>> On Sep 3, 2016, at 2:03 AM, CPC <acha...@gmail.com> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>> Cassandra driver 3.x require min guava 16.0.1. If it detects > an > >>>>>>>> earlier > >>>>>>>>>>>> version in classpath it stops working. > >>>>>>>>>>>> > >>>>>>>>>>>>> On Sep 3, 2016 04:26, "Julian Hyde" <jh...@apache.org> > wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>> James & Andrew, I hear you. We’ll stay on Guava 12 if we have > >>> to. > >>>>>>>>>>>>> > >>>>>>>>>>>>> But can we try an experiment to see if it’s possible to get > away > >>>>>>>> with > >>>>>>>>>>> 14? > >>>>>>>>>>>>> > >>>>>>>>>>>>> I propose that Maryann (who is developing the branch of > Phoenix > >>>>>> that > >>>>>>>>>>> uses > >>>>>>>>>>>>> Calcite) tries running with https://github.com/apache/ > >>>>>>>>> calcite/pull/277 > >>>>>>>>>>> < > >>>>>>>>>>>>> https://github.com/apache/calcite/pull/277>. If we discover > >>>>>>>> problems, > >>>>>>>>>>> we > >>>>>>>>>>>>> can try various solutions, like make the DateRangeRules > >>> disabled by > >>>>>>>>>>> default > >>>>>>>>>>>>> (these, and the Druid adapter, are the only parts of Calcite > >>> that > >>>>>>>> need > >>>>>>>>>>>>> Guava 14), or even copy the Guava classes that we need. If > there > >>>>>>>>> aren’t > >>>>>>>>>>>>> problems, it means that we’ve slipped out of the shackles of > >>>>>> inertia > >>>>>>>>>>> that > >>>>>>>>>>>>> are trying to drag us into an early grave. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Julian > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> On Sep 2, 2016, at 5:35 PM, James Taylor < > >>> jamestay...@apache.org> > >>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On the server-side, HBase depends on Guava 12 (because > Hadoop > >>>>>>>> depends > >>>>>>>>>>> on > >>>>>>>>>>>>>> the same). For that reason, we've made sure Phoenix can work > >>> with > >>>>>>>>> this > >>>>>>>>>>>>>> version too. Phoenix may not need to depend on Calcite on > the > >>>>>>>>>>>>> server-side, > >>>>>>>>>>>>>> and Phoenix and HBase both have shading, so there may be > some > >>>>>>>> avenues > >>>>>>>>>>> of > >>>>>>>>>>>>>> escape. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Sorry for the muddled answer. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Fri, Sep 2, 2016 at 5:21 PM, Andrew Purtell < > >>>>>>>> apurt...@apache.org > >>>>>>>>>> > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Use of Guava 14 introduces at least a compile time problem > >>> with > >>>>>>>>> HBase, > >>>>>>>>>>>>> upon > >>>>>>>>>>>>>>> which Phoenix depends, so I'm not sure Phoenix can move > off of > >>>>>> 13. > >>>>>>>>> I'd > >>>>>>>>>>>>> be > >>>>>>>>>>>>>>> happy to be proven wrong. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Fri, Sep 2, 2016 at 4:35 PM, Julian Hyde < > >>> jh...@apache.org> > >>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Calcite currently supports a wide range of Guava versions, > >>> from > >>>>>>>>>>> 12.0.1 > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>> 19.0*. For https://issues.apache.org/ > >>> jira/browse/CALCITE-1334 < > >>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/CALCITE-1334> I’d > >>> like to > >>>>>>>>> use > >>>>>>>>>>>>>>>> RangeSet, which was introduced in Guava 14. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Would anyone have a problem if we made Calcite’s minimum > >>> Guava > >>>>>>>>>>> version > >>>>>>>>>>>>>>>> 14.0.1? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> I see that Hive uses 14.0.1, Phoenix uses 13, Drill uses > 18. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Julian > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> * Except for the Druid adapter, which requires 14; see > >>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/CALCITE-1325 < > >>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/CALCITE-1325> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>> Best regards, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> - Andy > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Problems worthy of attack prove their worth by hitting > back. - > >>>>>>>> Piet > >>>>>>>>>>> Hein > >>>>>>>>>>>>>>> (via Tom White) > >>> > > -- Best regards, - Andy Problems worthy of attack prove their worth by hitting back. - Piet Hein (via Tom White)
