feat: Add Spark months_between function by zml1206 · Pull Request #14909 · facebookincubator/velox

zml1206 · 2025-09-19T09:28:38Z

Adds Spark months_between function.
https://github.com/apache/spark/blob/29434ea766b0fc3c3bf6eaadb43a8f931133649e/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala#L2031

netlify · 2025-09-19T09:28:44Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`d3caae8`
🔍 Latest deploy log	https://app.netlify.com/projects/meta-velox/deploys/68d5b7530aa2dc00086b9f63

jinchengchenghh · 2025-09-19T09:58:43Z

velox/docs/functions/spark/datetime.rst

+
+        SELECT months_between('1997-02-28 10:30:00', '1996-10-30', true); -- 3.94959677
+        SELECT months_between('1997-02-28 10:30:00', '1996-10-30', false); -- 3.9495967741935485
+        SELECT months_between('1997-02-28 10:30:00', '1996-03-31 11:00:00', true); -- 11


spark-sql (default)> SELECT months_between('1997-02-28 10:30:00', '1996-03-31 11:00:00', true); 11.0

jinchengchenghh · 2025-09-19T09:58:58Z

velox/docs/functions/spark/datetime.rst

+        SELECT months_between('1997-02-28 10:30:00', '1996-10-30', true); -- 3.94959677
+        SELECT months_between('1997-02-28 10:30:00', '1996-10-30', false); -- 3.9495967741935485
+        SELECT months_between('1997-02-28 10:30:00', '1996-03-31 11:00:00', true); -- 11
+        SELECT months_between('1997-02-21 10:30:00', '1996-03-21 11:00:00', true); -- 11


spark-sql (default)> SELECT months_between('1997-02-21 10:30:00', '1996-03-21 11:00:00', true);
11.0

jinchengchenghh · 2025-09-19T10:00:13Z

velox/functions/lib/TimeUtils.h

+inline constexpr int64_t kSecondsInMinute = 60;
+inline constexpr int64_t kSecondsInHour = 3600;
 inline constexpr int64_t kSecondsInDay = 86'400;
+inline constexpr int64_t kSecondsInMonth = 2'678'400;


Could you use a function such as kSecondsInDay * 31 to represent

jinchengchenghh · 2025-09-19T10:00:30Z

velox/functions/sparksql/DateTimeFunctions.h

  std::optional<DateTimeUnit> unit_ = std::nullopt;
 };

+template <typename T>


jinchengchenghh · 2025-09-19T10:02:22Z

velox/functions/lib/DateTimeUtil.h

 }

+FOLLY_ALWAYS_INLINE bool isEndDayOfMonth(const std::tm& tm) {
+  const auto endDay = util::getMaxDayOfMonth(getYear(tm), getMonth(tm));


remove endDay since it is used only once

jinchengchenghh · 2025-09-19T10:02:52Z

velox/functions/lib/DateTimeUtil.h

+}
+
+FOLLY_ALWAYS_INLINE double
+monthsBetween(const std::tm& tm1, const std::tm& tm2, const bool roundOff) {


const bool -> bool, remove cost when passing by value

jinchengchenghh · 2025-09-19T10:10:49Z

velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp

+        "months_between(c0, c1, c2)", timestamp1, timestamp2, roundOff);
+  };
+
+  EXPECT_EQ(


Could you add more roundOff false test to make sure it works well?

jinchengchenghh · 2025-09-19T19:23:33Z

velox/functions/lib/DateTimeUtil.h

+}
+
+FOLLY_ALWAYS_INLINE double
+monthsBetween(const std::tm& tm1, const std::tm& tm2, bool roundOff) {


Only SparkSql has this function, we could move it to struct MonthsBetweenFunction

jinchengchenghh

LGTM, could you also help take a look? Thanks! @rui-mo

zml1206 · 2025-09-24T05:55:38Z

Fuzzer test

rui-mo

Would you please update apache/gluten#10782 to ensure all Spark tests pass? Thanks.

velox/functions/lib/TimeUtils.h

velox/functions/sparksql/DateTimeFunctions.h

zml1206 · 2025-09-25T01:13:18Z

Would you please update apache/incubator-gluten#10782 to ensure all Spark tests pass?

It seems that all Spark tests passed, is there something I missed?

address comments

rui-mo · 2025-09-25T08:47:42Z

Would you please update apache/incubator-gluten#10782 to ensure all Spark tests pass?

It seems that all Spark tests passed, is there something I missed?

@zml1206 Just noticed some workflow failed, and want to make sure it works.

zml1206 · 2025-09-25T08:50:04Z

Would you please update apache/incubator-gluten#10782 to ensure all Spark tests pass?

It seems that all Spark tests passed, is there something I missed?

@zml1206 Just noticed some workflow failed, and want to make sure it works.

They are Enhanced, velox is a branch of IBM.

rui-mo

Thanks. Just two nits. Looks good overall.

velox/docs/functions/spark/datetime.rst

facebook-github-bot · 2025-09-29T17:26:18Z

@bikramSingh91 has imported this pull request. If you are a Meta employee, you can view this in D83490561.

facebook-github-bot · 2025-10-02T16:11:25Z

@bikramSingh91 merged this pull request in a6fa169.

feat: Add Spark months_between function

fe2e440

zml1206 requested review from jinchengchenghh, rui-mo and zhli1142015 as code owners September 19, 2025 09:28

meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 19, 2025

jinchengchenghh reviewed Sep 19, 2025

View reviewed changes

address comments

b047490

jinchengchenghh reviewed Sep 19, 2025

View reviewed changes

move to MonthsBetweenFunction

6e3dd9b

zml1206 mentioned this pull request Sep 23, 2025

[VL] Add months_between support apache/gluten#10781

Closed

jinchengchenghh approved these changes Sep 23, 2025

View reviewed changes

rui-mo reviewed Sep 24, 2025

View reviewed changes

velox/functions/lib/TimeUtils.h Outdated Show resolved Hide resolved

velox/functions/lib/TimeUtils.h Outdated Show resolved Hide resolved

rui-mo reviewed Sep 24, 2025

View reviewed changes

velox/functions/sparksql/DateTimeFunctions.h Outdated Show resolved Hide resolved

address comments

429b40c

address comments

zml1206 force-pushed the months_between branch from 2023776 to 429b40c Compare September 25, 2025 01:32

rui-mo reviewed Sep 25, 2025

View reviewed changes

velox/docs/functions/spark/datetime.rst Outdated Show resolved Hide resolved

velox/docs/functions/spark/datetime.rst Show resolved Hide resolved

update

d3caae8

rui-mo approved these changes Sep 26, 2025

View reviewed changes

rui-mo added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Sep 26, 2025

meta-codesync bot closed this in a6fa169 Oct 2, 2025

facebook-github-bot added the Merged label Oct 2, 2025

Conversation

zml1206 commented Sep 19, 2025

Uh oh!

netlify bot commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for meta-velox canceled.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinchengchenghh left a comment

Choose a reason for hiding this comment

Uh oh!

zml1206 commented Sep 24, 2025

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zml1206 commented Sep 25, 2025

Uh oh!

rui-mo commented Sep 25, 2025

Uh oh!

zml1206 commented Sep 25, 2025

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Sep 29, 2025

Uh oh!

facebook-github-bot commented Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

netlify bot commented Sep 19, 2025 •

edited

Loading