HeartSaVioR commented on issue #23749: [SPARK-26841][SQL] Kafka timestamp 
pushdown
URL: https://github.com/apache/spark/pull/23749#issuecomment-537237825
 
 
   I could help reviewing this, but as I'm not a committer you may still want 
to have attention from committers for this PR.
   
   First of all, you may want to rebase with latest master, and now you have a 
util to leverage when getting offset by timestamp, 
`KafkaOffsetReader.fetchSpecificTimestampBasedOffsets`. I guess it would reduce 
bunch of code lines here, making reviewers easier to review.
   
   Next, It feels the "Restriction" described above sounds a "correctness" 
issue instead of a restriction. Actually I don't see why it provides such 
result as it doesn't meet with javadoc in Kafka API.
   
   
https://kafka.apache.org/10/javadoc/org/apache/kafka/clients/consumer/KafkaConsumer.html#offsetsForTimes-java.util.Map-
   
   > Look up the offsets for the given partitions by timestamp. The returned 
offset for each partition is the earliest offset whose timestamp is greater 
than or equal to the given timestamp in the corresponding partition.
   
   Here it describes the case of "equal".

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to