[GitHub] [druid] suneet-s commented on a change in pull request #9530: DruidSegmentReader should work if timestamp is specified as a dimension

[GitBox <gi...@apache.org>]

suneet-s commented on a change in pull request #9530: DruidSegmentReader should work if timestamp is specified as a dimension
URL: https://github.com/apache/druid/pull/9530#discussion_r398084237
 
 

 ##########
 File path: indexing-service/src/main/java/org/apache/druid/indexing/input/DruidSegmentReader.java
 ##########
 @@ -270,6 +273,17 @@ public boolean hasNext()
           theEvent.put(metric, value);
         }
       }
+
+      /*
+       * Timestamp is added last because we expect that the time column will always be a date time object.
+       * If it is added earlier, it can be overwritten by metrics or dimenstions with the same name.
+       *
+       * If a user names a metric or dimension `__time` it will be overwritten. This case should be rare since
 
 Review comment:
   I'm worried about log explosion. since this is done per row. I'd have to add explicit checking outside of this next block. Maybe in the constructor? Would that be visible enough in the logs?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org