clintropolis commented on code in PR #16708: URL: https://github.com/apache/druid/pull/16708#discussion_r1669787476
########## processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java: ########## @@ -46,14 +46,16 @@ public class FixedIndexedWriter<T> implements DictionaryWriter<T> private final Comparator<T> comparator; private final ByteBuffer scratch; private final ByteBuffer readBuffer; - private int numWritten; + private final boolean isSorted; + private final int width; + + private int cardinality = 0; Review Comment: ah yea, this is just a nitpick, i was thinking it would be kind of nice if all of the `Indexed` writer implementations are consistent. `Indexed` itself however doesn't actually have a `getCardinality()`, it has a `size()` :D. it is only the `DictionaryWriter` interface that calls it cardinality which totally makes sense there, but the writers aren't always strictly writing dictionaries either, so :shrug: I don't feel super strongly either way other than consistent naming between them so its easier to look at implementations and see the differences while having a common language -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org