vlsi commented on a change in pull request #2590:
URL: https://github.com/apache/calcite/pull/2590#discussion_r731841508
##########
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -55,14 +55,14 @@
public class EnumerableValues extends Values implements EnumerableRel {
/** Creates an EnumerableValues. */
private EnumerableValues(RelOptCluster cluster, RelDataType rowType,
- ImmutableList<ImmutableList<RexLiteral>> tuples, RelTraitSet traitSet) {
+ List<? extends List<RexLiteral>> tuples, RelTraitSet traitSet) {
Review comment:
`List<RexLiteral>` allows clients to add items to the list.
However, `List<? extends RexLiteral>` is almost the same when it comes to
"reading from the list", however, one would not be able to add new values to
the list.
In other words, `List<? extends T>` is a more-or-less sane replacement for
`ImmutableList`.
Unfortunately, `List<? extends T>` would still allow methods like `.clear()`.
---
> remove guava classes from public API
Do you suggest completely removing guava dependency or do you suggest
removing it from public API only? What are the benefits?
I'm not sure it would be possible to fully eliminate Guava:
1) Guava would still be used for the implementation. Do we gain much if we
use Guava a lot for implementation purposes and try to avoid Guava elements in
API? `ImmutableList<ImmutableList<RexLiteral>>` is way easier to write and
maintain than `List<? extends List<? extends RexLiteral>>`. That is "no Guava
in API" has a maintenance cost.
2) There are cases like `Multimap` which have no standard Java classes. What
would you suggest for the replacement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
