sathyaprakashg commented on pull request #28222: URL: https://github.com/apache/spark/pull/28222#issuecomment-622256424
@cloud-fan @yaooqinn for great discussion on this. @yaooqinn has fixed getDays in the below PR and for getMonths we can agree on keeping the existing behavior as appropriate. https://github.com/apache/spark/pull/28396 I have another quick question, before we close this PR. SubtractTimestamps sems to be always populating only microseconds of CalendarInterval https://github.com/apache/spark/blob/b7cde42b04b21c9bfee6535199cf385855c15853/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala#L2242 Whereas SubtractDates seems to be populating months along with days field. https://github.com/apache/spark/blob/b7cde42b04b21c9bfee6535199cf385855c15853/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala#L2261 https://github.com/apache/spark/blob/ea525fe8c0cc6336a7ba8d98bada3198795f8aed/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala#L940 My question is whether we should change SubtractTimestamps also to populate appropraiate months and days fields as well in CalendarInterval or whether you guys feel existing implementation is appropriate ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
