morrySnow commented on PR #68096:
URL: https://github.com/apache/doris/pull/68096#issuecomment-5707588073

   > 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.
   
   You can download the pre-compiled version of thirdparty here. 
https://github.com/apache/doris-thirdparty/


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