Skip to content

Conversation

@charlesbluca
Copy link
Collaborator

Follow up to #300

In that PR, I assumed we could make the results of cross join operation consistent with the meta we construct for it by simply adding the common column we join on to the meta. I now understand that this doesn't work, as we don't necessarily know where the common column will be in the result, and the meta must have the same column ordering as its dataframe.

This PR resolves this by taking my original approach and just dropping the common column from the resulting dataframe altogether.

@charlesbluca charlesbluca changed the title Drop common column from result of cross join, remove from meta Drop common column from result of cross join, remove from corresponding meta Feb 23, 2022
@charlesbluca
Copy link
Collaborator Author

Think the failures here should be unblocked by #420

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022

Codecov Report

Merging #408 (48c74d3) into main (5beaf35) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
+ Coverage   89.03%   89.17%   +0.14%     
==========================================
  Files          69       69              
  Lines        3328     3327       -1     
  Branches      654      654              
==========================================
+ Hits         2963     2967       +4     
+ Misses        296      287       -9     
- Partials       69       73       +4     
Impacted Files Coverage Δ
dask_sql/physical/rel/logical/join.py 98.30% <100.00%> (-0.02%) ⬇️
dask_sql/_version.py 34.00% <0.00%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5beaf35...48c74d3. Read the comment docs.

@charlesbluca charlesbluca merged commit 8d88932 into dask-contrib:main Mar 24, 2022
@charlesbluca charlesbluca deleted the resolve-cross-join-meta branch July 19, 2022 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants