jadami10 commented on code in PR #12697: URL: https://github.com/apache/pinot/pull/12697#discussion_r1573344924
########## pinot-spi/src/main/java/org/apache/pinot/spi/stream/RowMetadata.java: ########## @@ -64,24 +62,34 @@ default long getFirstStreamRecordIngestionTimeMs() { } /** - * Returns the stream message headers - * - * @return A {@link GenericRow} that encapsulates the headers in the ingested row + * Returns the stream offset of the message. + */ + @Nullable Review Comment: out of curiosity, why do we use nulls and nullable annnotation instead of Optionals? ########## pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamDataDecoderImpl.java: ########## @@ -56,14 +56,15 @@ public StreamDataDecoderResult decode(StreamMessage message) { if (message.getKey() != null) { row.putValue(KEY, new String(message.getKey(), StandardCharsets.UTF_8)); } - RowMetadata metadata = message.getMetadata(); + StreamMessageMetadata metadata = message.getMetadata(); if (metadata != null) { if (metadata.getHeaders() != null) { metadata.getHeaders().getFieldToValueMap() .forEach((key, value) -> row.putValue(HEADER_KEY_PREFIX + key, value)); } - metadata.getRecordMetadata() - .forEach((key, value) -> row.putValue(METADATA_KEY_PREFIX + key, value)); + if (metadata.getRecordMetadata() != null) { Review Comment: maybe ``` Optional.ofNullable(metadata.getRecordMetadata()) .orElse(Collections.emptyMap()) ``` is a better pattern for the future so we don't call get twice -- 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...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org