fqaiser94 commented on a change in pull request #4009:
URL: https://github.com/apache/iceberg/pull/4009#discussion_r799485703
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/SparkTableUtil.java
##########
@@ -178,12 +179,16 @@ private SparkTableUtil() {
Option<scala.collection.immutable.Map<String, String>>
scalaPartitionFilter;
if (partitionFilter != null && !partitionFilter.isEmpty()) {
- scalaPartitionFilter =
Option.apply(JavaConverters.mapAsScalaMapConverter(partitionFilter).asScala()
- .toMap(Predef.conforms()));
+ Builder<Tuple2<String, String>, scala.collection.immutable.Map<String,
String>> builder =
+ Map$.MODULE$.<String, String>newBuilder();
+ for (Map.Entry<String, String> entry : partitionFilter.entrySet()) {
+ builder.$plus$eq(Tuple2.apply(entry.getKey(), entry.getValue()));
+ }
Review comment:
I fully agree with you that "using the Scala API in Java is really
ugly”, but in this case, I fear this is the better choice.
I thought it would be really simple, all we want to do is convert a java Map
into an *immutable* Scala map.
So, we start off by converting a java Map into a *mutable* Scala map:
> scala.collection.mutable.Map<String, String> myMutableMap =
JavaConverters.mapAsScalaMapConverter(myJavaMap).asScala();`
And now we can convert a *mutable* Scala map into an *immutable* Scala map
using the handy-dandy `.toMap` method.
But wait, the `.toMap` method takes an implicit argument `ev`:
> def toMap[K, V](implicit ev: A <:< (K, V)): immutable.Map[K, V]
This is basically a compiler trick to make sure we’re calling `.toMap` on a
collection of tuples (otherwise the method doesn’t make sense).
Normally, in Scala, this evidence is implicitly supplied.
Since we’re calling this method from Java, we don’t have the normal implicit
mechanisms available so we have to construct and supply this evidence by hand.
In the past we were able to supply this evidence by using
`scala.Predef.conforms()` but this API was removed entirely in 2.13 (it was
already deprecated in 2.12).
In scala 2.13, this implicit `ev` value of type `<:<` is provided by
`scala.Predef.refl()` in in Scala 2.13.
However, `scala.Predef.refl()` is not available in scala 2.12.
Right now, the only other alternative I can think of is having two separate
files for this code for 2.12 and 2.13, but that seems overkill for something so
small, hence I would argue this is the better choice.
Hope that makes sense. Open to thoughts.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
