Skip to content

[fix](function) Prevent cross-row rounding in months_between #67935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mryange merged 1 commit into from
Sep 15, 2026
+40 −64
Merged

[fix](function) Prevent cross-row rounding in months_between #67935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 21 additions & 58 deletions be/src/exprs/function/function_date_or_datetime_computation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "core/block/columns_with_type_and_name.h"
#include "core/column/column.h"
#include "core/column/column_const.h"
#include "core/column/column_execute_util.h"
#include "core/column/column_nullable.h"
#include "core/column/column_vector.h"
#include "core/data_type/data_type.h"
Expand Down Expand Up @@ -1682,77 +1683,39 @@ class FunctionMonthsBetween : public IFunction {
CHECK_EQ(arguments.size(), 3);
auto res = ColumnFloat64::create();

bool date_consts[2];
date_consts[0] = is_column_const(*block.get_by_position(arguments[0]).column);
date_consts[1] = is_column_const(*block.get_by_position(arguments[1]).column);
ColumnPtr date_cols[2];
// convert const columns to full columns if necessary
default_preprocess_parameter_columns(date_cols, date_consts, {0, 1}, block, arguments);

const auto& [col3, col3_const] =
unpack_if_const(block.get_by_position(arguments[2]).column);
const auto& round_off_col = *assert_cast<const ColumnBool*>(col3.get());

auto date_type = block.get_by_position(arguments[0]).type->get_primitive_type();
DORIS_CHECK_EQ(date_type, block.get_by_position(arguments[1]).type->get_primitive_type());
auto round_off =
ColumnView<TYPE_BOOLEAN>::create(block.get_by_position(arguments[2]).column);
if (date_type == TYPE_TIMESTAMP_NS) {
execute_typed<ColumnTimeStampNs>(input_rows_count, date_cols, date_consts, col3_const,
round_off_col, *res);
auto date1 = ColumnView<TYPE_TIMESTAMP_NS>::create(
block.get_by_position(arguments[0]).column);
auto date2 = ColumnView<TYPE_TIMESTAMP_NS>::create(
block.get_by_position(arguments[1]).column);
execute_typed(input_rows_count, date1, date2, round_off, *res);
} else {
DORIS_CHECK_EQ(date_type, TYPE_DATEV2);
execute_typed<ColumnDateV2>(input_rows_count, date_cols, date_consts, col3_const,
round_off_col, *res);
auto date1 =
ColumnView<TYPE_DATEV2>::create(block.get_by_position(arguments[0]).column);
auto date2 =
ColumnView<TYPE_DATEV2>::create(block.get_by_position(arguments[1]).column);
execute_typed(input_rows_count, date1, date2, round_off, *res);
}

block.replace_by_position(result, std::move(res));
return Status::OK();
}

private:
template <typename DateColumn>
static void execute_typed(size_t input_rows_count, const ColumnPtr (&date_cols)[2],
const bool (&date_consts)[2], bool round_off_const,
const ColumnBool& round_off_col, ColumnFloat64& res) {
const auto& date1_col = *assert_cast<const DateColumn*>(date_cols[0].get());
const auto& date2_col = *assert_cast<const DateColumn*>(date_cols[1].get());
if (date_consts[0] && date_consts[1]) {
execute_vector<true, false>(input_rows_count, date1_col, date2_col, round_off_col, res);
} else if (round_off_const) {
execute_vector<false, true>(input_rows_count, date1_col, date2_col, round_off_col, res);
} else {
execute_vector<false, false>(input_rows_count, date1_col, date2_col, round_off_col,
res);
}
}

template <bool is_date_const, bool is_round_off_const, typename DateColumn>
static void execute_vector(const size_t input_rows_count, const DateColumn& date1_col,
const DateColumn& date2_col, const ColumnBool& round_off_col,
ColumnFloat64& res) {
template <PrimitiveType DateType>
static void execute_typed(size_t input_rows_count, const ColumnView<DateType>& date1,
const ColumnView<DateType>& date2,
const ColumnView<TYPE_BOOLEAN>& round_off, ColumnFloat64& res) {
res.reserve(input_rows_count);
double months_between;
bool round_off;

if constexpr (is_date_const) {
auto dtv1 = date_v2_from_date_like(date1_col.get_element(0));
auto dtv2 = date_v2_from_date_like(date2_col.get_element(0));
months_between = calc_months_between(dtv1, dtv2);
}

if constexpr (is_round_off_const) {
round_off = round_off_col.get_element(0);
}

for (int i = 0; i < input_rows_count; ++i) {
if constexpr (!is_date_const) {
auto dtv1 = date_v2_from_date_like(date1_col.get_element(i));
auto dtv2 = date_v2_from_date_like(date2_col.get_element(i));
months_between = calc_months_between(dtv1, dtv2);
}
if constexpr (!is_round_off_const) {
round_off = round_off_col.get_element(i);
}
if (round_off) {
Comment thread
Mryange marked this conversation as resolved.
for (size_t i = 0; i < input_rows_count; ++i) {
auto months_between = calc_months_between(date_v2_from_date_like(date1.value_at(i)),
date_v2_from_date_like(date2.value_at(i)));
if (round_off.value_at(i)) {
months_between = round_months_between(months_between);
}
res.insert_value(months_between);
Expand Down
24 changes: 18 additions & 6 deletions regression-test/data/query_p0/sql_functions/string_functions/test_months_between.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@
-1 -1 -1 -1
-1.90322581 -0.03225806 -1.90322581 -0.03225806
-1.90322581 0.90322581 \N \N
-12.96774194 \N -12.96774193548387 \N
-13.87096774 0.87096774 -13.87096774193548 \N
-12.96774194 \N -12.967741935483872 \N

@github-actions github-actions Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Avoid changing unaffected unrounded results

These oracle changes come from calls with only one constant date (the queries at Groovy lines 79-83), so they can never enter the buggy date_consts[0] && date_consts[1] path. The rewrite nevertheless changes the returned binary64 values: for example, the old -12.96774193548387 is the nearest double to exact -402/31, while the new -12.967741935483872 is the adjacent one-ULP value; 340/31 changes the same way. This is unrelated numerical compatibility drift, and some cases become less accurate. Please keep the asymmetric/vector calculation path unchanged and narrowly fix the both-date-const path by copying an immutable precomputed raw value before per-row rounding, then restore the unaffected expected results.

-13.87096774 0.87096774 -13.870967741935484 \N
-6 \N \N 4.93548387

-- !const_other_not_nullable --
Expand All @@ -122,8 +122,8 @@
-1.4516129 -0.5483871 -1.4516129 -0.5483871
-1.90322581 -0.03225806 -1.90322581 -0.03225806
-1.90322581 0.90322581 -1.903225806451613 0.9032258064516129
-12.96774194 10.96774194 -12.96774193548387 10.96774193548387
-13.87096774 0.87096774 -13.87096774193548 0.8709677419354839
-12.96774194 10.96774194 -12.967741935483872 10.967741935483872
-13.87096774 0.87096774 -13.870967741935484 0.8709677419354839
-6 4.93548387 -6 4.93548387
-971.96774194 -1441 -971.96774194 -1441

Expand Down Expand Up @@ -158,8 +158,8 @@
-1.4516129 -1.4516129
-1.90322581 -1.903225806451613
-1.90322581 -1.90322581
-12.96774194 -12.96774193548387
-13.87096774 -13.87096774193548
-12.96774194 -12.967741935483872
-13.87096774 -13.870967741935484
-6 -6
-971.96774194 -971.96774194

Expand All @@ -175,6 +175,18 @@
-1
-1

-- !const_dates_round_off --
1 true 2.03225806
10 false 2.032258064516129
2 false 2.032258064516129
3 true 2.03225806
4 false 2.032258064516129
5 true 2.03225806
6 false 2.032258064516129
7 true 2.03225806
8 false 2.032258064516129
9 true 2.03225806

-- !const23 --
-0.03225806
-0.5483871
Expand Down
1 change: 1 addition & 0 deletions regression-test/suites/query_p0/sql_functions/string_functions/test_months_between.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ suite("test_months_between") {
order_qt_const_partial_nullable_no_null "select months_between('2020-01-01', nullable('2020-02-01')), months_between(nullable('2020-01-01'), '2020-02-01'), months_between('2020-01-01', nullable('2020-02-01 00:00:00'), nullable(true)) from months_between_args"
order_qt_const1 "select months_between('2020-01-01', date2_not_null), months_between('2020-01-01', date2_not_null, round_off_not_null) from months_between_args"
order_qt_const12 "select months_between('2020-01-01', '2020-02-01', round_off_not_null) from months_between_args"
order_qt_const_dates_round_off "select k0, round_off_not_null, months_between('2020-12-26', '2020-10-25', round_off_not_null) from months_between_args order by k0"
order_qt_const23 "select months_between(date1_not_null, '2020-02-01', true) from months_between_args"
order_qt_const3 "select months_between(date1_not_null, date2_not_null, true) from months_between_args"

Expand Down
Loading