github-actions[bot] commented on code in PR #68410:
URL: https://github.com/apache/doris/pull/68410#discussion_r4080005127


##########
be/test/exprs/function/function_time_test.cpp:
##########
@@ -36,6 +37,37 @@
 namespace doris {
 using namespace ut_type;
 
+template <typename Transform>
+void check_quarter_interval_overflow(const typename Transform::InputValueType& 
date) {
+    SCOPED_TRACE(Transform::name);
+    SCOPED_TRACE(Transform::ArgPType);
+    // These intervals can wrap to small month offsets if multiplication uses 
Int32.
+    for (Int32 quarters :
+         {1431655765, 1431655766, -1431655765, -1431655766, 715827882, 
715827883, -715827882,
+          -715827883, std::numeric_limits<Int32>::min(), 
std::numeric_limits<Int32>::max()}) {
+        SCOPED_TRACE(quarters);
+        EXPECT_THROW(Transform::execute(date, quarters), Exception);

Review Comment:
   [P2] Make the INT_MIN subtraction check sign-sensitive. `EXPECT_THROW` 
accepts both the intended +6442450944-month path and a narrowed/wrapped 
-6442450944-month path because both are out of range, so the exact negation 
regression called out in `AddQuartersImpl` could return while this test remains 
green. Please assert the positive widened delta in the `SubtractQuartersImpl` 
exception (or test a small conversion helper); the existing small-wrap cases 
can continue covering multiplication overflow.



##########
regression-test/suites/query_p0/sql_functions/datetime_functions/test_quarters_add.groovy:
##########
@@ -216,4 +216,61 @@ suite("test_quarters_add") {
             assertTrue(exception != null)
         }
     }
-}
\ No newline at end of file
+
+    // Cover both small wrapped results and the Int32 multiplication/negation 
boundaries.
+    def intervals = [1431655765, 1431655766, -1431655765, -1431655766,
+                     715827882, 715827883, -715827882, -715827883,
+                     -2147483648, 2147483647, 40000, -40000]
+    def types = ["date", "datetime(6)", "timestamptz(6)", "timestamp_ns"]
+    for (def skipFold : [false, true]) {

Review Comment:
   [P2] Trim the redundant P0 error matrix. These loops add 192 constant 
queries, 96 table queries, and four boundary queries, while the BE unit test 
already runs the full numeric boundary set through all eight transforms. Please 
keep the old small-wrap values and a minimal set covering FE fold, BE 
constant/vector execution, the four result types, and add/sub, rather than 
paying 292 sequential failing-query round trips in every P0 run.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to