jihoonson commented on a change in pull request #12250:
URL: https://github.com/apache/druid/pull/12250#discussion_r812479358
##########
File path:
processing/src/main/java/org/apache/druid/segment/serde/ColumnPartSerde.java
##########
@@ -39,13 +41,23 @@
@JsonSubTypes.Type(name = "floatV2", value =
FloatNumericColumnPartSerdeV2.class),
@JsonSubTypes.Type(name = "longV2", value =
LongNumericColumnPartSerdeV2.class),
@JsonSubTypes.Type(name = "doubleV2", value =
DoubleNumericColumnPartSerdeV2.class),
+ @JsonSubTypes.Type(name = "null", value = NullColumnPartSerde.class)
})
public interface ColumnPartSerde
{
@Nullable
Serializer getSerializer();
- Deserializer getDeserializer();
+ /**
+ * Returns a Deserializer to read a column from a segment.
+ *
+ * @param rowCountSupplier a supplier that returns the row count of
the segment that the column belongs to.
+ * Getting from the supplier the row count
can be expensive and thus should be
+ * evaluated lazily.
+ * @param segmentBitmapSerdeFactory bitmapSerdeFactory stored in the
segment. Each columnPartSerde may have their own
+ * bitmapSerdeFactory.
+ */
+ Deserializer getDeserializer(IntSupplier rowCountSupplier,
BitmapSerdeFactory segmentBitmapSerdeFactory);
Review comment:
So, here are the reasons that I modified this interface.
- I added `NullColumnPartSerde` as a default type of `ColumnPartSerde`. This
will be useful when the historical tries to deserialize a `ColumnPartSerde`
that is unknown to the version of the historical. The historical will use
`NullColumnPartSerde` instead of exploding.
- Since `NullColumnPartSerde` is the default type of `ColumnPartSerde`, its
constructor should be simple with no parameters. The fallback to
`NullColumnPartSerde` for unknown `ColumnPartSerde` will fail otherwise.
- To create a bitmapIndex for null columns, `NullColumnPartSerde` needs to
know the number of rows in the segment and the bitmap index type.
I agree that the current interface change is specific to this PR which could
be better if it is more generic. I have reverted this particular interface
change in this PR but am going to make another PR for it. In this PR, [I made
the null column
_not-filterable_](https://github.com/apache/druid/pull/12250/files#diff-753cc3c64ec70b8ca1be35049eb1273be6403fd3ab447ba6bdc0e234c75646b1R91-R94),
so that `ColumnSelectorBitmapIndexSelector` can create one for it instead.
This is a hacky workaround to prioritize fixing `getDeserializer` interface.
--
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]
