Fix perf-compare script and related perf README #5435
Merged
+118
−118
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philrz
changed the title
nwt
approved these changes
Nov 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's Changing
The
perf-compare.shscript is updated to use the new|>pipe symbol. The performance README that contains a recent output of this script has also been updated to reflect the new super-era file format names.Why
After the merge of #5427, the continued use of the plain
|pipe symbol inperf-compare.shcaused this CI failure, so these changes get that to run clean again. While in there, I also saw some other super-fying changes worth making.Details
When I last updated
perf-compare.shthe new format names were still in flux, so the perf README table still had references to zng/zson/etc., but now that we've changed all thesupercommand flags and file extensions referenced in the docs to refer to new foramts things seem pretty settled, so I've updated the perf README to reflect that.I also recently had a chat with @mccanne and we agreed that talking about "SuperPipe" like it's a wholly separate thing is likely to be phased out, so I took the liberty of tweaking the variable names in
perf-compare.shto reflect that. Specifically, we've already had some rough consensus about.spqas a file extension that would give a hint to a user that it contains SuperSQL that uses some amount of pipeline-based extensions (i.e., spq = "SuperSQL Pipe Query"), and so in the script I'm trying this out.