calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Purtell <apurt...@apache.org>
Subject Re: Dropping support for Guava versions earlier than 14
Date Tue, 06 Sep 2016 19:04:06 GMT
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