What about APIs with byte[] parameters? Lot's of APIs have this, but almost all of the time the byte[] is immutable. That's kind of in the same category as what-if String were mutable, no?
On Tue, Sep 6, 2016 at 2:11 PM, Julian Hyde <jh...@apache.org> wrote: > 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) > >> > >> > >
