gianm commented on code in PR #14563:
URL: https://github.com/apache/druid/pull/14563#discussion_r1261331564


##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -238,75 +232,243 @@ static int getNumberOfFilesRequired(int bagSize, long 
numWritten)
     return numberOfFilesRequired;
   }
 
+  protected final ObjectStrategy<T> strategy;
+  protected final boolean allowReverseLookup;
+  protected final int size;
 
-  private final boolean versionOne;
+  public GenericIndexed(
+      final ObjectStrategy<T> strategy,
+      final boolean allowReverseLookup,
+      final int size
+  )
+  {
+    this.strategy = strategy;
+    this.allowReverseLookup = allowReverseLookup;
+    this.size = size;
+  }
 
-  private final ObjectStrategy<T> strategy;
-  private final boolean allowReverseLookup;
-  private final int size;
-  private final ByteBuffer headerBuffer;
+  public abstract BufferIndexed singleThreaded();
 
-  private final ByteBuffer firstValueBuffer;
+  @Override
+  public abstract long getSerializedSize();
+
+  private static final class V1<T> extends GenericIndexed<T>

Review Comment:
   Yeah it has certainly grown complicated over time. Probably 
over-complicated. I am guessing you are still OK with merging this patch, given 
the ✅, but I would support efforts to make the class less complicated.



##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -238,75 +232,243 @@ static int getNumberOfFilesRequired(int bagSize, long 
numWritten)
     return numberOfFilesRequired;
   }
 
+  protected final ObjectStrategy<T> strategy;
+  protected final boolean allowReverseLookup;
+  protected final int size;
 
-  private final boolean versionOne;
+  public GenericIndexed(
+      final ObjectStrategy<T> strategy,
+      final boolean allowReverseLookup,
+      final int size
+  )
+  {
+    this.strategy = strategy;
+    this.allowReverseLookup = allowReverseLookup;
+    this.size = size;
+  }
 
-  private final ObjectStrategy<T> strategy;
-  private final boolean allowReverseLookup;
-  private final int size;
-  private final ByteBuffer headerBuffer;
+  public abstract BufferIndexed singleThreaded();
 
-  private final ByteBuffer firstValueBuffer;
+  @Override
+  public abstract long getSerializedSize();
+
+  private static final class V1<T> extends GenericIndexed<T>

Review Comment:
   Yeah it has certainly grown complicated over time. Probably 
over-complicated. I am guessing you are still OK with merging this patch, given 
the ✅, but I would support efforts to make the class less complicated.



-- 
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]

Reply via email to