HappenLee commented on PR #66477:
URL: https://github.com/apache/doris/pull/66477#issuecomment-5695534849

   **[P1] Reject IDENTITY plans when the configured execution version is below 
15**
   
   At commit `8774b5d426d4477a5f2d822201ec9509218af858`, 
[Config](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-common/src/main/java/org/apache/doris/common/Config.java#L2011-L2026)
 declares `DISTRIBUTION_HASH_TYPE_MIN_BE_EXEC_VERSION = 15` and raises the 
default maximum version, but this constant is not used to gate IDENTITY plans. 
`be_exec_version` can still be configured to 14, while 
[DataPartition.toThrift()](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-core/src/main/java/org/apache/doris/planner/DataPartition.java#L118-L135)
 and 
[OlapTableSink](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java#L1002)
 still emit IDENTITY metadata.
   
   This matters when a new FE runs with a BE that predates this feature and the 
compatibility execution version is set to 14:
   
   1. Create an IDENTITY table with a BIGINT distribution key and 8 buckets.
   2. Load `id = 1` through the old BE. It ignores the new optional Thrift 
field and continues using CRC32 routing.
   3. CRC32 of the 8-byte little-endian BIGINT value `1`, modulo 8, is **7**; 
IDENTITY modulo 8 is **1**.
   4. The new FE prunes `WHERE id = 1` to bucket 1 even though the old BE 
routed the row to bucket 7. This can make committed data disappear from a 
pruned query.
   
   Raising the default version to 15 lets an old BE reject the default new 
plan, but it does not protect the explicitly configured version-14 path. The 
stream-load planner also takes its execution version from 
`Config.be_exec_version`.
   
   Validation: a focused FE test sets the execution version to 14 and 
serializes a real `DataPartition(BUCKET_SHFFULE_HASH_PARTITIONED, ..., 
IDENTITY)`. Serialization succeeds; the assertion expecting a compatibility 
rejection fails because no exception is thrown. The bucket-number difference 
was checked separately. I have not run a mixed-version cluster to reproduce the 
write/pruning mismatch end to end; that consequence follows from the old/new 
routing and protocol behavior.
   
   Please enforce the minimum supported execution version for IDENTITY across 
writes and query/exchange plans. DDL validation can provide an earlier error, 
but cannot be the only check because the execution-version configuration may 
change later. Existing TIMESTAMP_NS version validation in 
`PrimitiveType.toThrift()` provides a precedent. Silently falling back to CRC32 
would change the persisted table's distribution contract and is not a safe 
compatibility strategy.
   


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