zsxwing commented on a change in pull request #26134: [SPARK-29486][SQL] 
CalendarInterval should have 3 fields: months, days and microseconds
URL: https://github.com/apache/spark/pull/26134#discussion_r350926544
 
 

 ##########
 File path: 
common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java
 ##########
 @@ -31,45 +32,50 @@
   public static final long MICROS_PER_WEEK = MICROS_PER_DAY * 7;
 
   public final int months;
+  public final int days;
   public final long microseconds;
 
   public long milliseconds() {
     return this.microseconds / MICROS_PER_MILLI;
   }
 
-  public CalendarInterval(int months, long microseconds) {
+  public CalendarInterval(int months, int days, long microseconds) {
 
 Review comment:
   @LinhongLiu Could you send out a follow up PR to document why we need `days` 
and how do we use it in this file? This is definitely worth to document. 
Otherwise, everyone reading this class may need to git blame and go through the 
long discussion in this PR.

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