lets-order-some-fries commented on PR #68096:
URL: https://github.com/apache/doris/pull/68096#issuecomment-5700937138

   One thing a reviewer should know before spending CI on this: **I could not 
build the FE locally**, so
   `run buildall` will be the first compile of this change.
   
   What I did verify:
   
   - `mvn -pl fe-core checkstyle:check` passes (the same command CI runs).
   - Every symbol the diff uses, read against the source at `19ebe13d`:
     `IntegerLikeLiteral.getBigDecimalValue()` is declared on the class the 
cast targets, and
     `LargeIntLiteral` overrides it (it overrides `getNumber` and 
`getBigDecimalValue`, but **not**
     `getLongValue` — which is why the comparison uses the `BigDecimal` form).
   - The three `bindWithOrdinal` call sites, and that the narrowing happens 
before the range test.
   
   What I could not verify: that it compiles and that the added test passes. 
Building the FE here needs
   Doris's `thirdparty/installed` toolchain, which this machine does not have, 
and the fallback of
   compiling `fe-core` against previously installed artifacts fails for an 
unrelated reason — the
   installed poms carry an unresolved `${revision}`, so sibling descriptors 
cannot be read.
   
   I would rather say that up front than have it surface as a red buildall. If 
the compile or the test
   does fail, tell me and I will fix it immediately.
   


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