alamb commented on code in PR #23259:
URL: https://github.com/apache/datafusion/pull/23259#discussion_r3500552482


##########
datafusion/physical-expr/src/partitioning.rs:
##########
@@ -593,11 +600,18 @@ pub enum Distribution {
     UnspecifiedDistribution,
     /// A single partition is required
     SinglePartition,
+    /// Deprecated historical name for [`Distribution::KeyPartitioned`].

Review Comment:
   I also suggest adding a link to the discussion ticket for rationale
   ```rust
   // see https://github.com/apache/datafusion/issues/23236 for details
   ```



##########
datafusion/physical-expr/src/partitioning.rs:
##########
@@ -593,11 +600,18 @@ pub enum Distribution {
     UnspecifiedDistribution,
     /// A single partition is required
     SinglePartition,
+    /// Deprecated historical name for [`Distribution::KeyPartitioned`].
+    #[deprecated(since = "54.0.0", note = "Use Distribution::KeyPartitioned")]

Review Comment:
   Yes, I think it should be 55 -- tracking in 
   - https://github.com/apache/datafusion/issues/22393



##########
datafusion/physical-expr/src/partitioning.rs:
##########
@@ -799,43 +825,66 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    #[expect(
+        deprecated,
+        reason = "test intentionally covers deprecated HashPartitioned 
compatibility"
+    )]
+    fn deprecated_hash_partitioned_matches_key_partitioned() -> Result<()> {

Review Comment:
   this is good for me too



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