calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Taylor <jamestay...@apache.org>
Subject Re: Dropping support for Guava versions earlier than 14
Date Tue, 06 Sep 2016 21:25:18 GMT
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 <jhyde@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.dunning@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 <jhyde@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 <apurtell@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 <jhyde@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.purtell@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 <jacques@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 <jhyde@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.purtell@gmail.com> wrote:
> >>>>>>>> Use hbase-shaded-client as Maven dep (1.1 and up)
> >>>>>>>>
> >>>>>>>>> On Sep 3, 2016, at 10:12 AM, James Taylor <
> jamestaylor@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.purtell@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
<
> >> jamestaylor@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 <
> >>>> jacques@apache.org>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> +1 on shading guava/protobuf.
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Sat, Sep 3, 2016 at 9:48 AM, Andrew
Purtell <
> >>>>>>>>>> andrew.purtell@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 <
> >> jacques@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.purtell@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 <achalil@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" <jhyde@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 <
> >>>>>>> jamestaylor@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 <
> >>>>>>>>>>>> apurtell@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 <
> >>>>>>> jhyde@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)
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message