yx-keith commented on PR #68096:
URL: https://github.com/apache/doris/pull/68096#issuecomment-5772714666

   LGTM. 
   
   The fix is correct: `IntegerLikeLiteral.getBigDecimalValue()` defaults to 
`getLongValue()` and only `LargeIntLiteral` overrides it, so BigDecimal is the 
only width-safe comparison here.
   
   Two nits:
   1. `LogicalPlanBuilder.convertSortOrdinalsToUnboundSlot` (DELETE/UPDATE 
ORDER BY) has the same truncation: `getIntValue()` turns `order by 4294967297` 
into `UnboundSlot("1")`. Worth fixing here too.
   2. `assertThrows(Exception.class)` passes on any exception. Please assert 
the error message, and cover the two ORDER BY call sites (they don't throw) by 
checking the sort keys in the plan.
   


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