(from the peanut gallery)
I have no strong opinions but find myself presently leaning towards
Ted's suggestions.
Ideally, I think that we should not expose things in a "public API"
which we do not have the ability to guarantee compatibility of. A
round-about way of saying "Calcite public API should only be Calcite
classes or Java 'standard library' classes". Included Guava classes in
the API would be disallowed. The basic reasoning behind this is that it
guarantees that we have full control of our "destiny" without having to
worry about dependency versions.
On the quick read of Ted's suggestion, it matches that (hide the Guava
class in the API's implementation with a generic type in the API). But,
I can also appreciate the problems you outline as well, Julian.
Julian Hyde wrote:
Yes, I misread your suggestion. I see now you are suggesting a compromise. The
worst that can happen is that the API users make a few unnecessary defensive
copies, or due to missing documentation they have to spend a little longer
researching the API in order to discover that it is “safe”.
I am not particularly inclined to compromise, because I would like Java to be a
high level language. An immutable collection is inherently simpler than a
mutable collection. Consider the problem of contravariance[1] in generics: if I
have a function
int countLegs(List<? extends Animal> animals)
and I pass into it an argument of type
List<? extends Dog>
then the Java compiler rejects it, because it thinks that the function might
try to add a Cat to the list. But we know that this is impossible, because
countLegs does not modify the list. If immutability is part of the declared
type of the list, the type validation rules can be looser.
Julian
[1]
https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science)<https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science)>
On Sep 6, 2016, at 2:41 PM, Ted Dunning<ted.dunn...@gmail.com> wrote:
Julian,
I think you miss the point of my suggestion.
String is not called ImmutableString. The contract of no immutability is
carried by outside tribal knowledge and documentation. It is enforced by
not allowing mutation.
If you declare a List, but assign an ImmutableList, you are in a similar
situation except that the tribal knowledge has to be more forceful because
of the normal expectation of List as mutable.
On Wed, Sep 7, 2016 at 7:11 AM, 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)
