How bad would it be for API designers and users if java.lang.String were mutable? I would say really, really bad. You could add a lot of comments to the API documentation, but you’d never really be sure that everyone was adhering to the contract.
> On Sep 6, 2016, at 1:59 PM, Ted Dunning <ted.dunn...@gmail.com> wrote: > > What is so bad about declaring that variable as a List and making it an > ImmutableList underneath? > > Guard it in the programmer's mind by comments and naming. And if they don't > believe you, it still can't be changed. > > This avoids Guava leakage in the API and still gives you (nearly) all of > the benefits of the ImmutableList type. > > Kind of give a little to get a little. > > > > On Wed, Sep 7, 2016 at 5:10 AM, Julian Hyde <jh...@apache.org> wrote: > >> What is so bad about Guava? I have always found it to be a high quality >> library. I hear that they have broken backwards compatibility on one or two >> occasions, but I’ve never been affected by that personally. >> >>> On Sep 6, 2016, at 12:04 PM, Andrew Purtell <apurt...@apache.org> wrote: >>> >>> 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) >> >>
