Skip to content

Add a UUID extension type#6832

Open
connortsui20 wants to merge 4 commits intodevelopfrom
ct/uuid
Open

Add a UUID extension type#6832
connortsui20 wants to merge 4 commits intodevelopfrom
ct/uuid

Conversation

@connortsui20
Copy link
Contributor

@connortsui20 connortsui20 commented Mar 6, 2026

Summary

Adds a UUID extension type to Vortex over a FixedSizeList<u8, 16>. There isn't much functionality implemented on top of this, and there is no exporter to Arrow's UUID canonical extension type. All of the logic is delegated to the Rust uuid crate.

There is still a question of if we want to bring in FixedSizeBinary instead of doing this.

Testing

Some small basic tests. They serve mostly as example code on how to work with Uuid extension arrays in general.

Open Questions

  • Do we actually need FixedSizeBinary instead here? Why does this not suffice?
  • Is there actually an alignment issue here?
  • The performance of unpack_native is terrible because we still do not have ScalarValue::Array (see ScalarValue::Array #6717)
  • Should we name this something other than vortex.uuid since we know we might change the storage type in the future?
  • UuidFromString should really be implemented under the Cast expression, but we do not have a system yet for pluggable expressions so it is not entirely clear how we would do that.
  • What other expressions should we have on Uuid? UuidToString is an easy next one, what else is there?

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 requested review from asubiotto and gatesn March 6, 2026 19:52
@connortsui20 connortsui20 added the changelog/feature A new feature label Mar 6, 2026
@connortsui20 connortsui20 marked this pull request as ready for review March 6, 2026 19:58
elements.len()
);

let mut bytes = [0u8; UUID_BYTE_LEN];
Copy link
Contributor

Choose a reason for hiding this comment

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

🤮 - can't wait for #6717

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😭

/// Accepts any standard UUID string format (hyphenated, simple, braced, URN). Invalid strings
/// cause an error.
#[derive(Clone)]
pub struct UuidFromString;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I really do think we should just go directly to pluggable casting. CC @joseph-isaacs

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 6, 2026

Merging this PR will degrade performance by 11%

❌ 2 regressed benchmarks
✅ 168 untouched benchmarks
⏩ 2276 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation map_each[BufferMut<i32>, 128] 770.6 ns 858.1 ns -10.2%
Simulation bitwise_not_vortex_buffer_mut[128] 471.9 ns 530.3 ns -11%

Comparing ct/uuid (32ccf9f) with develop (5d6a3c8)2

Open in CodSpeed

Footnotes

  1. 2276 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on develop (6a34208) during the generation of this report, so 5d6a3c8 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@connortsui20 connortsui20 marked this pull request as draft March 6, 2026 20:09
@connortsui20
Copy link
Contributor Author

Ok so I'm going to remove the UuidFromString scalar fn and add a Option<Version> using https://docs.rs/uuid/latest/uuid/enum.Version.html as metadata

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 marked this pull request as ready for review March 6, 2026 21:47
@connortsui20 connortsui20 force-pushed the ct/uuid branch 2 times, most recently from 127212c to 9242442 Compare March 6, 2026 22:08
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants