Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary sorts before joins in ztests #4784

Merged
merged 2 commits into from
Sep 28, 2023

Conversation

nwt
Copy link
Member

@nwt nwt commented Sep 27, 2023

Remove sort operators before joins in ztests that are unnecessary now that join automatically sorts its inputs.

compiler/ztests/join-desc.yaml is left unmodified since it is explicitly testing inputs with descending order.

zfmt/ztests/decls.yaml is modified for consistency, even though removing the sorts affects the output (which is formatted Zed, not query results).

Remove sort operators before joins in ztests that are unnecessary now
that join automatically sorts its inputs.

compiler/ztests/join-desc.yaml is left unmodified since it is explicitly
testing inputs with descending order.

zfmt/ztests/decls.yaml is modified for consistency, even though removing
the sorts affects the output (which is formatted Zed, not query
results).
@nwt nwt requested a review from a team September 27, 2023 14:31
Comment on lines 3 to 9
zq -z 'anti join (file B.zson) on a=b | sort a' A.zson
zq -z 'anti join (file B.zson) on a=b' A.zson
echo === LEFT ===
zq -z 'left join (file B.zson) on a=b hit:=sb | sort a' A.zson
zq -z 'left join (file B.zson) on a=b hit:=sb' A.zson
echo === INNER ===
zq -z 'inner join (file B.zson) on a=b hit:=sb | sort a' A.zson
zq -z 'inner join (file B.zson) on a=b hit:=sb' A.zson
echo === RIGHT ===
zq -z 'right join (file C.zson) on b=c hit:=sb | sort c' B.zson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing, I happened to notice that the original actually looks broken here, since the | sort are being tacked on after the join. It looks like it only worked consistently because the inputs happened to be pre-sorted by the respective keys.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted! I'll back out the change to this file since these sorts aren't before the join.

@nwt nwt merged commit 7c1aaa2 into main Sep 28, 2023
3 checks passed
@nwt nwt deleted the remove-unnecessary-sorts-in-ztests branch September 28, 2023 15:06
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.

3 participants