tarun11Mavani commented on code in PR #19093:
URL: https://github.com/apache/pinot/pull/19093#discussion_r3975793110


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java:
##########
@@ -152,6 +156,62 @@ public JsonIndexReader getSparseJsonIndex() {
     return _sparseDataSource != null ? _sparseDataSource.getJsonIndex() : null;
   }
 
+  @SuppressWarnings("unchecked")
+  @Nullable
+  @Override
+  public Map<String, Object> getMapValue(int docId) {

Review Comment:
   No live path today crashes on this specifically — `getPhysicalColumnNames()` 
is schema-derived and doesn't surface materialized children when an explicit 
schema is supplied. But loading a segment *without* one (e.g. the deprecated 
`PinotSegmentRecordReader(File)` ctor) makes `SegmentMetadataImpl` self-derive 
a schema from every physical column on disk, materialized children included — 
that path did crash. Added the `isMaterializedChild()` skip in 
`addColumnReader` you suggested, plus a sealed-segment round-trip test 
(`ImmutableOpenStructDataSourceTest#testSealedSegmentRoundTripThroughRecordReader`)
 that exercises it for real instead of through mocks.



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