You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "mkleen (via GitHub)" <gi...@apache.org> on 2026/07/24 09:53:26 UTC
[PR] Add slt for swapped cross join with conflicting metadata [datafusion]
mkleen opened a new pull request, #23857:
URL: https://github.com/apache/datafusion/pull/23857
## Which issue does this PR close?
Follow-up for https://github.com/apache/datafusion/pull/23605 based on the [feedback](https://github.com/apache/datafusion/pull/23605#issuecomment-5065282520) from @2010YOUY01
## Rationale for this change
This adds a slt test for https://github.com/apache/datafusion/pull/23605 to make sure it works end-to-end.
## What changes are included in this PR?
- slt test
- data creation in `test_context.rs`
## Are these changes tested?
Yes
## Are there any user-facing changes?
No
--
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] Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen commented on code in PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#discussion_r3644496913
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -520,3 +520,11 @@ NULL the id field
statement ok
drop table table_with_metadata;
+
+# Test that swapped cross-join preserves metadata on conflicting values on metadata.
+# The larger_table has 10 values, smaller_tables 1 value and the fields of each table
+# have conflicting metadata, same key different values See test:context.rs register_conflicting_metadata_tables
+query II
+select * from larger_table cross join smaller_table order by left.a limit 1;
Review Comment:
This fails without https://github.com/apache/datafusion/pull/23605
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#issuecomment-5082530613
Thanks for the reproducer!
I tend to think we might want to let this test case fail, here are more details in the original issue https://github.com/apache/datafusion/issues/23461, we could continue the discussion there.
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#issuecomment-5069339159
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/23857?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 80.70%. Comparing base ([`1c8295c`](https://app.codecov.io/gh/apache/datafusion/commit/1c8295c992c597a53dcfa01f6dcf1cdc51268adb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ad64982`](https://app.codecov.io/gh/apache/datafusion/commit/ad64982613be71d139866d5b31291c6e87235358?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 21 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #23857 +/- ##
==========================================
- Coverage 80.75% 80.70% -0.05%
==========================================
Files 1089 1090 +1
Lines 368809 370168 +1359
Branches 368809 370168 +1359
==========================================
+ Hits 297837 298754 +917
- Misses 53217 53600 +383
- Partials 17755 17814 +59
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/23857?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen commented on PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#issuecomment-5068585031
However, this just proves that the cross-join is working now, the same issue could be at a lot of other code parts two where schemas get merged.
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen commented on PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#issuecomment-5129412326
Closed by https://github.com/apache/datafusion/pull/23952
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#discussion_r3664469087
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -520,3 +520,11 @@ NULL the id field
statement ok
drop table table_with_metadata;
+
+# Test that swapped cross-join preserves metadata on conflicting values on metadata.
Review Comment:
Nice regression test! One small thing I noticed is that the setup attaches the conflicting `metadata_key` to the Arrow `Schema`, but this comment says the fields have conflicting metadata. The query also only verifies the returned values. Would it make sense to either update the comment to explain that this is a planner/runtime regression where conflicting schema metadata should not cause a swapped cross join to fail, or add an assertion with `arrow_metadata(...)` if preserving field metadata is the behaviour you want to verify?
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen commented on code in PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#discussion_r3644496913
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -520,3 +520,11 @@ NULL the id field
statement ok
drop table table_with_metadata;
+
+# Test that swapped cross-join preserves metadata on conflicting values on metadata.
+# The larger_table has 10 values, smaller_tables 1 value and the fields of each table
+# have conflicting metadata, same key different values See test:context.rs register_conflicting_metadata_tables
+query II
+select * from larger_table cross join smaller_table order by left.a limit 1;
Review Comment:
This fails without https://github.com/apache/datafusion/pull/23605
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen commented on PR #23857:
URL: https://github.com/apache/datafusion/pull/23857#issuecomment-5069116675
Test fails with https://github.com/apache/datafusion/pull/23605
--
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] test: Add slt for swapped cross join with conflicting metadata [datafusion]
Posted by "mkleen (via GitHub)" <gi...@apache.org>.
mkleen closed pull request #23857: test: Add slt for swapped cross join with conflicting metadata
URL: https://github.com/apache/datafusion/pull/23857
--
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