parentheses value should support alias #496 #578
Merged
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
This patch support alias for parentheses value. e.g) select (1 + 2) as v from access Currently, parentheses are mandatory.
pauldix
added a commit
that referenced
this pull request
May 26, 2014
…support-alias parentheses value should support alias #496
|
Thanks @chobie! |
devanbenz
pushed a commit
that referenced
this pull request
Dec 5, 2024
* fix(influxd): update xxhash, avoid stringtoslicebyte in cache This commit does 3 things: * it updates xxhash from v1 to v2; v2 includes a assembly arm version of Sum64 * it changes the cache storer to write with a string key instead of a byte slice. The cache only reads the key which WriteMulti already has as a string so we can avoid a host of allocations when converting back and forth from immutable strings to mutable byte slices. This includes updating the cache ring and ring partition to write with a string key * it updates the xxhash for finding the cache ring partition to use Sum64String which uses unsafe pointers to directly use a string as a byte slice since it only reads the string. Note: this now uses an assembly version because of the v2 xxhash update. Go 1.22 included new compiler ability to recognize calls of Method([]byte(myString)) and not make a copy but from looking at the call sites, I'm not sure the compiler would recognize it as the conversion to a byte slice was happening several calls earlier. That's what this change set does. If we are uncomfortable with any of these, we can do fewer of them (for example, not upgrade xxhash; and/or not use the specialized Sum64String, etc). For the performance issue in maz-rr, I see converting string keys to byte slices taking between 3-5% of cpu usage on both the primary and secondary. So while this pr doesn't address directly the increased cpu usage on the secondary, it makes cpu usage less on both which still feels like a win. I believe these changes are easier to review that switching to a byte slice pool that is likely needed in other places as the compiler provides nearly all of the correctness checks we need (we are relying also on xxhash v2 being correct). * helps #550 * chore: fix tests/lint * chore: don't use assembly version; should inline This 2 line change causes xxhash to use a purego Sum64 implementation which allows the compiler to see that Sum64 only read the byte slice input which them means is can skip the string to byte slice allocation and since it can skip that, it should inline all the calls to getPartitionStringKey and Sum64 avoiding 1 call to Sum64String which isn't inlined. * chore: update ci build file the ci build doesn't use the make file!!! * chore: revert "chore: update ci build file" This reverts commit 94be66fde03e0bbe18004aab25c0e19051406de2. * chore: revert "chore: don't use assembly version; should inline" This reverts commit 67d8d06c02e17e91ba643a2991e30a49308a5283. (cherry picked from commit 1d334c679ca025645ed93518b7832ae676499cd2)
devanbenz
added a commit
that referenced
this pull request
Dec 5, 2024
…25622) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache (#578) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache This commit does 3 things: * it updates xxhash from v1 to v2; v2 includes a assembly arm version of Sum64 * it changes the cache storer to write with a string key instead of a byte slice. The cache only reads the key which WriteMulti already has as a string so we can avoid a host of allocations when converting back and forth from immutable strings to mutable byte slices. This includes updating the cache ring and ring partition to write with a string key * it updates the xxhash for finding the cache ring partition to use Sum64String which uses unsafe pointers to directly use a string as a byte slice since it only reads the string. Note: this now uses an assembly version because of the v2 xxhash update. Go 1.22 included new compiler ability to recognize calls of Method([]byte(myString)) and not make a copy but from looking at the call sites, I'm not sure the compiler would recognize it as the conversion to a byte slice was happening several calls earlier. That's what this change set does. If we are uncomfortable with any of these, we can do fewer of them (for example, not upgrade xxhash; and/or not use the specialized Sum64String, etc). For the performance issue in maz-rr, I see converting string keys to byte slices taking between 3-5% of cpu usage on both the primary and secondary. So while this pr doesn't address directly the increased cpu usage on the secondary, it makes cpu usage less on both which still feels like a win. I believe these changes are easier to review that switching to a byte slice pool that is likely needed in other places as the compiler provides nearly all of the correctness checks we need (we are relying also on xxhash v2 being correct). * helps #550 * chore: fix tests/lint * chore: don't use assembly version; should inline This 2 line change causes xxhash to use a purego Sum64 implementation which allows the compiler to see that Sum64 only read the byte slice input which them means is can skip the string to byte slice allocation and since it can skip that, it should inline all the calls to getPartitionStringKey and Sum64 avoiding 1 call to Sum64String which isn't inlined. * chore: update ci build file the ci build doesn't use the make file!!! * chore: revert "chore: update ci build file" This reverts commit 94be66fde03e0bbe18004aab25c0e19051406de2. * chore: revert "chore: don't use assembly version; should inline" This reverts commit 67d8d06c02e17e91ba643a2991e30a49308a5283. (cherry picked from commit 1d334c679ca025645ed93518b7832ae676499cd2) * feat: need to update go sum --------- Co-authored-by: Phil Bracikowski <13472206+philjb@users.noreply.github.com>
devanbenz
added a commit
that referenced
this pull request
Dec 6, 2024
…25622) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache (#578) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache This commit does 3 things: * it updates xxhash from v1 to v2; v2 includes a assembly arm version of Sum64 * it changes the cache storer to write with a string key instead of a byte slice. The cache only reads the key which WriteMulti already has as a string so we can avoid a host of allocations when converting back and forth from immutable strings to mutable byte slices. This includes updating the cache ring and ring partition to write with a string key * it updates the xxhash for finding the cache ring partition to use Sum64String which uses unsafe pointers to directly use a string as a byte slice since it only reads the string. Note: this now uses an assembly version because of the v2 xxhash update. Go 1.22 included new compiler ability to recognize calls of Method([]byte(myString)) and not make a copy but from looking at the call sites, I'm not sure the compiler would recognize it as the conversion to a byte slice was happening several calls earlier. That's what this change set does. If we are uncomfortable with any of these, we can do fewer of them (for example, not upgrade xxhash; and/or not use the specialized Sum64String, etc). For the performance issue in maz-rr, I see converting string keys to byte slices taking between 3-5% of cpu usage on both the primary and secondary. So while this pr doesn't address directly the increased cpu usage on the secondary, it makes cpu usage less on both which still feels like a win. I believe these changes are easier to review that switching to a byte slice pool that is likely needed in other places as the compiler provides nearly all of the correctness checks we need (we are relying also on xxhash v2 being correct). * helps #550 * chore: fix tests/lint * chore: don't use assembly version; should inline This 2 line change causes xxhash to use a purego Sum64 implementation which allows the compiler to see that Sum64 only read the byte slice input which them means is can skip the string to byte slice allocation and since it can skip that, it should inline all the calls to getPartitionStringKey and Sum64 avoiding 1 call to Sum64String which isn't inlined. * chore: update ci build file the ci build doesn't use the make file!!! * chore: revert "chore: update ci build file" This reverts commit 94be66fde03e0bbe18004aab25c0e19051406de2. * chore: revert "chore: don't use assembly version; should inline" This reverts commit 67d8d06c02e17e91ba643a2991e30a49308a5283. (cherry picked from commit 1d334c679ca025645ed93518b7832ae676499cd2) * feat: need to update go sum --------- Co-authored-by: Phil Bracikowski <13472206+philjb@users.noreply.github.com> (cherry picked from commit 06ab224)
devanbenz
added a commit
that referenced
this pull request
Dec 6, 2024
…25622) (#25624) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache (#578) * fix(influxd): update xxhash, avoid stringtoslicebyte in cache This commit does 3 things: * it updates xxhash from v1 to v2; v2 includes a assembly arm version of Sum64 * it changes the cache storer to write with a string key instead of a byte slice. The cache only reads the key which WriteMulti already has as a string so we can avoid a host of allocations when converting back and forth from immutable strings to mutable byte slices. This includes updating the cache ring and ring partition to write with a string key * it updates the xxhash for finding the cache ring partition to use Sum64String which uses unsafe pointers to directly use a string as a byte slice since it only reads the string. Note: this now uses an assembly version because of the v2 xxhash update. Go 1.22 included new compiler ability to recognize calls of Method([]byte(myString)) and not make a copy but from looking at the call sites, I'm not sure the compiler would recognize it as the conversion to a byte slice was happening several calls earlier. That's what this change set does. If we are uncomfortable with any of these, we can do fewer of them (for example, not upgrade xxhash; and/or not use the specialized Sum64String, etc). For the performance issue in maz-rr, I see converting string keys to byte slices taking between 3-5% of cpu usage on both the primary and secondary. So while this pr doesn't address directly the increased cpu usage on the secondary, it makes cpu usage less on both which still feels like a win. I believe these changes are easier to review that switching to a byte slice pool that is likely needed in other places as the compiler provides nearly all of the correctness checks we need (we are relying also on xxhash v2 being correct). * helps #550 * chore: fix tests/lint * chore: don't use assembly version; should inline This 2 line change causes xxhash to use a purego Sum64 implementation which allows the compiler to see that Sum64 only read the byte slice input which them means is can skip the string to byte slice allocation and since it can skip that, it should inline all the calls to getPartitionStringKey and Sum64 avoiding 1 call to Sum64String which isn't inlined. * chore: update ci build file the ci build doesn't use the make file!!! * chore: revert "chore: update ci build file" This reverts commit 94be66fde03e0bbe18004aab25c0e19051406de2. * chore: revert "chore: don't use assembly version; should inline" This reverts commit 67d8d06c02e17e91ba643a2991e30a49308a5283. (cherry picked from commit 1d334c679ca025645ed93518b7832ae676499cd2) * feat: need to update go sum --------- Co-authored-by: Phil Bracikowski <13472206+philjb@users.noreply.github.com> (cherry picked from commit 06ab224)
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.
This patch support alias for parentheses value.
e.g)
select (1 + 2) as v from access
Currently, parentheses are mandatory.