amogh-jahagirdar commented on code in PR #7353:
URL: https://github.com/apache/iceberg/pull/7353#discussion_r1167668817
##########
core/src/main/java/org/apache/iceberg/LocationProviders.java:
##########
@@ -109,7 +109,7 @@ static class ObjectStoreLocationProvider implements
LocationProvider {
private static final HashFunction HASH_FUNC = Hashing.murmur3_32_fixed();
private static final BaseEncoding BASE64_ENCODER =
BaseEncoding.base64Url().omitPadding();
- private final ThreadLocal<byte[]> temp = ThreadLocal.withInitial(() -> new
byte[4]);
+ private transient ThreadLocal<byte[]> temp;
Review Comment:
I think we can have a `static` threadlocal and keep the initialization as is
(and remove 175-177) to keep the change a bit simpler?
Static fields (like transient) are also not serialized. Let me know if I'm
missing something
##########
spark/v3.3/spark/src/test/java/org/apache/iceberg/TestTableSerialization.java:
##########
@@ -33,11 +33,25 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+@RunWith(Parameterized.class)
public class TestTableSerialization {
+ public TestTableSerialization(String isObjectStoreEnabled) {
+ this.isObjectStoreEnabled = isObjectStoreEnabled;
+ }
+
+ @Parameterized.Parameters(name = "isObjectStoreEnabled = {0}")
+ public static Object[] parameters() {
+ return new Object[] {"true", "false"};
Review Comment:
Do we need to parameterize? I suppose it's fine because there's very few
tests in the class, and they're all pretty lightweight so the runtime of the
tests won't increase much. I was thinking we can just add a separate test where
object store is set to true
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
