You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/09/20 22:18:27 UTC

[PR] WIP: Account for constant equivalence properties in union, tests [datafusion]

alamb opened a new pull request, #12562:
URL: https://github.com/apache/datafusion/pull/12562

   Draft as I haven't actually fixed the bug yet, I am just writing tests
   
   ## Which issue does this PR close?
   
   
   Closes https://github.com/apache/datafusion/issues/12446
   Closes https://github.com/apache/datafusion/pull/12414
   
   
   ## Rationale for this change
   
   Fixes a bug: https://github.com/apache/datafusion/issues/12446
   
   ## What changes are included in this PR?
   
   TBD
   (I hope to break the comments / test refactor out into separate PRs for easier review)
   
   ## Are these changes tested?
   Yes
   
   ## Are there any user-facing changes?
   
   Fixed bug
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] WIP: Account for constant equivalence properties in union, tests [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12562:
URL: https://github.com/apache/datafusion/pull/12562#discussion_r1769323644


##########
datafusion/physical-expr/src/equivalence/properties.rs:
##########
@@ -2617,379 +2633,356 @@ mod tests {
         ))
     }
 
-    #[tokio::test]
-    async fn test_union_equivalence_properties_multi_children() -> Result<()> {
-        let schema = create_test_schema()?;
+    #[test]
+    fn test_union_equivalence_properties_multi_children_1() {
+        let schema = create_test_schema().unwrap();
         let schema2 = append_fields(&schema, "1");
         let schema3 = append_fields(&schema, "2");
-        let test_cases = vec![
-            // --------- TEST CASE 1 ----------
-            (
+        UnionEquivalenceTest::new(&schema)

Review Comment:
   I wanted more flexibility with the tests, and I am glad I did this as I hadn't seen the existing tests for constants



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org