Conversation
Merging this PR will degrade performance by 10.2%
Performance Changes
Comparing Footnotes
|
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
c0b0735 to
1625151
Compare
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
| uses: ./.github/actions/setup-rust | ||
| with: | ||
| repo-token: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Install uv |
There was a problem hiding this comment.
We set up uv to upload coverage results, worth removing it from there to avoid duplicate installation
There was a problem hiding this comment.
that's a different job IMO, runs on a different machine
| ((high as i128) << 64) | (low as i128) | ||
| } | ||
|
|
||
| #[inline] |
There was a problem hiding this comment.
Maybe worth defining in #[tests]?
There was a problem hiding this comment.
We use it in ExtractedValue
| @@ -0,0 +1,32 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Not a blocker, but I'd avoid using bash if we can go with sh.
I see only &> (can be replaced with 1>/dev/null 2>&1) and
BASH_SOURCE. Replacing the second one is more tricky but I think something like $_
would work.
| vortex_bail!("DuckDB HugeInt is not yet supported in Vortex"); | ||
| } | ||
| ExtractedValue::UHugeInt(_) => { | ||
| vortex_bail!("DuckDB UHugeInt is not yet supported in Vortex"); |
There was a problem hiding this comment.
is this u128 int? We might add this soon.
Good to know duckdb has it
There was a problem hiding this comment.
yeah duckdb seems to have both
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
This PR adds TPC-H to to our SLT suite, it requires generating data (script provided), and currently doesn't work due to subtle formatting differences between the crates.
One reason for those differences is that we actually run the queries on different schemas as they each infer a different schema.
I've also added some duckdb functionality as I tried to get it working, and I figure its worth merging it anyway.