Skip to content
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.

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

Fix bug of datetime64 parsed from string '1969-12-31 23:59:59.123' #37039

Merged
merged 8 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Core/DecimalFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ inline DecimalComponents<DecimalType> splitWithScaleMultiplier(
using T = typename DecimalType::NativeType;
const auto whole = decimal.value / scale_multiplier;
auto fractional = decimal.value % scale_multiplier;
if (fractional < T(0))
if (whole && fractional < T(0))
fractional *= T(-1);

return {whole, fractional};
Expand Down Expand Up @@ -199,7 +199,7 @@ inline typename DecimalType::NativeType getFractionalPartWithScaleMultiplier(
/// Anycase we make modulo before compare to make scale_multiplier > 1 unaffected.
T result = decimal.value % scale_multiplier;
if constexpr (!keep_sign)
if (result < T(0))
if (decimal.value / scale_multiplier && result < T(0))
result = -result;

return result;
Expand Down
30 changes: 29 additions & 1 deletion src/Core/tests/gtest_DecimalFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,24 @@ INSTANTIATE_TEST_SUITE_P(Basic,
}
},
{
"When scale is not 0 and whole part is 0.",
"For positive Decimal value, with scale not 0, and whole part is 0.",
123,
3,
{
0,
123
}
},
{
"For negative Decimal value, with scale not 0, and whole part is 0.",
-123,
3,
{
0,
-123
}
},

{
"For negative Decimal value whole part is negative, fractional is non-negative.",
-1234567'89,
Expand Down Expand Up @@ -216,6 +226,24 @@ INSTANTIATE_TEST_SUITE_P(Basic,
187618332,
123
}
},
{
"Negative timestamp 1969-12-31 23:59:59.123 UTC",
DateTime64(-877),
3,
{
0,
-877
}
},
{
"Positive timestamp 1970-01-01 00:00:00.123 UTC",
DateTime64(123),
3,
{
0,
123
}
}
})
);
21 changes: 19 additions & 2 deletions src/IO/ReadHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -931,12 +931,29 @@ inline ReturnType readDateTimeTextImpl(DateTime64 & datetime64, UInt32 scale, Re
++buf.position();

/// Keep sign of fractional part the same with whole part if datetime64 is negative
/// 1965-12-12 12:12:12.123 => whole = -127914468, fraction = 123(sign>0) -> new whole = -127914467, new fraction = 877(sign<0)
/// Case1:
/// 1965-12-12 12:12:12.123
/// => whole = -127914468, fractional = 123(coefficient>0)
/// => new whole = -127914467, new fractional = 877(coefficient<0)
///
/// Case2:
/// 1969-12-31 23:59:59.123
/// => whole = -1, fractional = 123(coefficient>0)
/// => new whole = 0, new fractional = -877(coefficient>0)
if (components.whole < 0 && components.fractional != 0)
{
const auto scale_multiplier = DecimalUtils::scaleMultiplier<DateTime64::NativeType>(scale);
++components.whole;
components.fractional = scale_multiplier - components.fractional;
if (components.whole)
{
/// whole keep the sign, fractional should be non-negative
components.fractional = scale_multiplier - components.fractional;
}
else
{
/// when whole is zero, fractional should keep the sign
components.fractional = components.fractional - scale_multiplier;
}
}
}
/// 9908870400 is time_t value for 2184-01-01 UTC (a bit over the last year supported by DateTime64)
Expand Down
21 changes: 18 additions & 3 deletions src/IO/WriteHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -805,11 +805,21 @@ inline void writeDateTimeText(DateTime64 datetime64, UInt32 scale, WriteBuffer &
scale = scale > MaxScale ? MaxScale : scale;

auto components = DecimalUtils::split(datetime64, scale);
/// -127914467.877 => whole = -127914467, fraction = 877 => new whole = -127914468(1965-12-12 12:12:12), new fraction = 123(.123) => 1965-12-12 12:12:12.123
if (components.whole < 0 && components.fractional != 0)
/// Case1:
/// -127914467.877
/// => whole = -127914467, fraction = 877(After DecimalUtils::split)
/// => new whole = -127914468(1965-12-12 12:12:12), new fraction = 1000 - 877 = 123(.123)
/// => 1965-12-12 12:12:12.123
///
/// Case2:
/// -0.877
/// => whole = 0, fractional = -877(After DecimalUtils::split)
/// => whole = -1(1969-12-31 23:59:59), fractional = 1000 + (-877) = 123(.123)
using T = typename DateTime64::NativeType;
if (datetime64.value < 0 && components.fractional)
{
components.fractional = DecimalUtils::scaleMultiplier<T>(scale) + (components.whole ? T(-1) : T(1)) * components.fractional;
--components.whole;
components.fractional = DecimalUtils::scaleMultiplier<DateTime64::NativeType>(scale) - components.fractional;
}

writeDateTimeText<date_delimeter, time_delimeter, between_date_time_delimiter>(LocalDateTime(components.whole, time_zone), buf);
Expand Down Expand Up @@ -989,7 +999,12 @@ void writeText(Decimal<T> x, UInt32 scale, WriteBuffer & ostr, bool trailing_zer
{
part = DecimalUtils::getFractionalPart(x, scale);
if (part || trailing_zeros)
{
if (part < 0)
part *= T(-1);

writeDecimalFractional(part, scale, ostr, trailing_zeros);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
-127914467.877
187618332.123
1969-12-31 23:59:59.123
1 change: 1 addition & 0 deletions tests/queries/0_stateless/02242_negative_datetime64.sql
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
SELECT cast(toDateTime64('1965-12-12 12:12:12.123', 3, 'UTC') as Decimal64(3));
SELECT cast(toDateTime64('1975-12-12 12:12:12.123', 3, 'UTC') as Decimal64(3));
SELECT toDateTime64('1969-12-31 23:59:59.123', 3, 'UTC');