Doris-Breakwater commented on issue #66649:
URL: https://github.com/apache/doris/issues/66649#issuecomment-5249626291

   Breakwater-GitHub-Analysis-Slot: slot_bf41f3fe5fab
   
   ## Initial triage
   
   **Assessment: likely valid correctness defect, but the report itself is 
incomplete.** The evidence points to Nereids implicit comparison coercion 
rather than the hash-join implementation. This should be triaged as an 
incorrect-result issue in the Nereids/type-coercion area. The issue currently 
has no labels, assignee, milestone, usable version, reproduction, actual 
result, or concrete expected result.
   
   ### Verified from current `master`
   
   At upstream `master` commit 
[`165efa32a9d54ac575cb8403a4dd7d6273c52de2`](https://github.com/apache/doris/commit/165efa32a9d54ac575cb8403a4dd7d6273c52de2):
   
   - Join predicates are analyzed as normal comparison predicates through 
[`ExpressionAnalyzer.visitComparisonPredicate`](https://github.com/apache/doris/blob/165efa32a9d54ac575cb8403a4dd7d6273c52de2/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java#L868-L875).
   - With the new coercion behavior, a fixed numeric value compared with a 
string is assigned `DECIMAL(38, decimal_overflow_scale)`, or `DECIMAL(76, 
decimal_overflow_scale)` when Decimal256 is enabled; both operands are then 
cast to that common type ([common-type 
selection](https://github.com/apache/doris/blob/165efa32a9d54ac575cb8403a4dd7d6273c52de2/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java#L1070-L1095),
 [comparison 
casts](https://github.com/apache/doris/blob/165efa32a9d54ac575cb8403a4dd7d6273c52de2/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java#L1341-L1365)).
 The default `decimal_overflow_scale` is `6` 
([`SessionVariable`](https://github.com/apache/doris/blob/165efa32a9d54ac575cb8403a4dd7d6273c52de2/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java#L1963-L1968)).
   - The BE decimal parser rounds half up when fractional digits exceed the 
target scale 
([`string_parser.cpp`](https://github.com/apache/doris/blob/165efa32a9d54ac575cb8403a4dd7d6273c52de2/be/src/util/string_parser.cpp#L234-L258)).
 Therefore distinct exact values can collapse to the same comparison key. For 
example, under the default scale, `DECIMAL(38,7) 100.1234564` and VARCHAR text 
`100.12345649` both become `DECIMAL(38,6) 100.123456`, producing a false 
equality match. Conversely, converting a full-width `DECIMAL(38,0)` to 
`DECIMAL(38,6)` reduces integer capacity and can make an exact large value and 
its identical text fail to match.
   - With legacy coercion behavior, DECIMAL/string comparison falls back to 
`DOUBLE`, which can also lose precision for sufficiently large or precise 
values.
   
   This verifies a deterministic mechanism capable of producing both false 
matches and missed matches. It does **not** verify that the reporter's 
unprovided query/version reached exactly this path.
   
   There is already an open implementation attempt, [PR 
#65693](https://github.com/apache/doris/pull/65693), for the same behavior. Its 
current head is `18ad9ebad643e1e9a3c1549788a4f9d0afb2cf75` from July 17 and it 
substantially predates current `master`. Review threads still identify 
unresolved correctness boundaries, including conversion rejection versus SQL 
`NULL`, strict-cast behavior, prepared/external execution paths, and 
complex/nested comparisons. It should be linked to this issue and rebased, but 
should not be considered a completed fix yet.
   
   ### Information required from the reporter
   
   Please provide:
   
   1. Exact Doris version/build (`SELECT VERSION()` plus the FE/BE build commit 
if available). The current **Version** field repeats the title and is not a 
version.
   2. Minimal DDL and inserts for both join columns, including the exact 
`DECIMAL(p,s)`, VARCHAR definition, nullability, and representative values.
   3. The smallest failing SQL, actual rows/count, and expected rows/count. 
Preserve the VARCHAR values exactly, including trailing digits, signs, 
whitespace, or scientific notation.
   4. `EXPLAIN VERBOSE` for that SQL and these settings: 
`enable_new_type_coercion_behavior`, `enable_decimal256`, 
`decimal_overflow_scale`, and `enable_strict_cast`.
   5. Whether this is ordinary `=`, null-safe `<=>`, a range predicate, `IN`, 
or another predicate; and whether either side is an external/remote table or a 
complex type.
   
   A query profile is not needed for initial correctness confirmation; the 
analyzed cast types in `EXPLAIN VERBOSE` are more useful. FE/BE logs are needed 
only if the query errors or a cast/overflow warning is observed.
   
   ### Recommended next steps
   
   1. Add the relevant bug/correctness and Nereids/type-coercion component 
labels, then request the missing reproduction and version details.
   2. Link PR #65693 and update its issue reference to this issue if it remains 
the chosen implementation.
   3. Before merge, rebase the PR and resolve its existing correctness review 
threads. Explicitly define whether the fix is limited to ordinary equality 
joins or is intended to cover `<=>`, `IN`/`NOT IN`, range predicates, prepared 
statements, external pushdown, and nested types.
   4. Require regression coverage for both coercion modes, Decimal256 on/off, 
source scales above `decimal_overflow_scale`, full-width DECIMAL integer 
boundaries, invalid strings, NULLs, strict-cast mode, and both join-side 
orientations.
   


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