Improve Varbinary Hex Functions#14889
Conversation
pettyjamesm
force-pushed
the
improve-varbinary-functions
branch
2 times, most recently
from
3ccf8f6 to
36994ba
Compare
There was a problem hiding this comment.
Currently trino-spi doesn't have junit on the classpath for testing dependencies and all the other tests are using org.testng.annotations.Test. Are we trying to switch the preferred annotations for new PR's over to junit instead?
There was a problem hiding this comment.
We’re slowly trying to migrate to JUnit (#9378) so I think we should try to take the opportunity to use it when adding new tests. Otherwise, it’s more work someone has to do later.
There was a problem hiding this comment.
Got it- good to know. I tried switching just this one test to use jUnit, but it seemed like adding junit to trino-spi/pom.xml prevented all of the existing testng tests from running. A quick google search seemed to expose ways to run jUnit tests within TestNG, but I couldn't find anything about doing it the other way around- so it seems like using jUnit here would require migrating all of the trino-spi tests all at once, unless you know of a way to avoid it.
There was a problem hiding this comment.
Nevermind, I see that trino-main/pom.xml has a configuration for allowing both to co-exist. I'll push a new commit in this PR to add jUnit support to trino-spi and migrate this new test to use those annotations instead.
pettyjamesm
force-pushed
the
improve-varbinary-functions
branch
from
36994ba to
36267eb
Compare
Improves to_hex() and from_hex() functions performance by removing extra String -> byte[] round-trips and operating over the underlying byte arrays directly where possible.
Improves performance of convering SqlVarbinary values to Strings by precomputing the exact string output size and avoiding an inner loop call to String.format per byte. Also switches SqlVarbinary#compareTo implementation to use the Arrays.compare directly.
pettyjamesm
force-pushed
the
improve-varbinary-functions
branch
from
36267eb to
d2af93c
Compare
2 participants
Description
This PR contains three minor improvements, in separate commits:
VarbinaryFunctions.{toHex,fromHexVarchar}implementations by avoiding intermediateStringmaterializations and operating over the primitive byte arrays where possible.VarbinaryFunctions.crc32to use the primitive byte array when possible instead of creating a newByteBufferto compute the crc32 valueSqlVarbinary#toStringby precomputing the exact output size required for theStringBuilderand usingjava.util.HexFormatinstead of callingString.formatas part of the per-byte inner loop.Non-technical explanation
No non-technical explanation is necessary, this are not user visible changes.
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: