It isn't in the same category because nothing forces immutability. If Calcite were to return an ImmutableList declared as a List, then it would still be immutable (i.e. would throw if mutation is attempted).
The byte[] can't do this. On Wed, Sep 7, 2016 at 7:25 AM, James Taylor <jamestay...@apache.org> wrote: > 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) > > >> > > >> > > > > >
