Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add lt, lte, gt, gte, neq, between for UUID type #10584

Closed
aditi-pandit opened this issue Jul 26, 2024 · 8 comments
Closed

Add lt, lte, gt, gte, neq, between for UUID type #10584

aditi-pandit opened this issue Jul 26, 2024 · 8 comments
Assignees
@BryanCutler
Labels
enhancement New feature or request

Comments

@aditi-pandit
Copy link
Collaborator

Description

Presto UUID type supports neq, lt, lte, gt, gte, between operations UUIDOperators

This allows more flexible use in SQL.

The lack of these operators causes exceptions today
https://github.com/prestodb/presto/pull/23301/files#r1693434020

Since we support equality on UUID, it seems strange not to support the rest of these operations.
@aditi-pandit aditi-pandit added the enhancement New feature or request label Jul 26, 2024
@aditi-pandit
Copy link
Collaborator Author

@mbasmanova : Any particular reason we don't support these ? Don't think this has the logical type comparison issue as Timestamp with Timezone.

@BryanCutler
Copy link
Contributor

Hi @aditi-pandit I'll work on this one

@mbasmanova
Copy link
Contributor

@aditi-pandit No particular reason. Let's add missing functionality.

@BryanCutler Thank you for offering to work on this.

@mbasmanova
Copy link
Contributor

CC: @amitkdutta

@aditi-pandit aditi-pandit mentioned this issue Jul 29, 2024
Draft
@BryanCutler
Copy link
Contributor

Just a quick update - I have the basic implementation done for comparisons, working on adding tests and will put up a PR soon.

@BryanCutler BryanCutler mentioned this issue Aug 20, 2024
Closed
facebook-github-bot pushed a commit that referenced this issue Jan 11, 2025
@BryanCutler @facebook-github-bot
feat: Add support for UUID comparison functions (#10791)
022cd87 
Summary:
This adds binary comparison functions <, >, <=, >= to the UUID custom data type. Equality functions are already present. Added unit tests for testing a query with comparisons between UUID values.

The ordering is done lexicographically to conforms with IETF RFC 4122 https://datatracker.ietf.org/doc/html/rfc4122.html, and also matches Presto Java after prestodb/presto#23311.

Related UUID serialization fix at #11197
From #10584

Pull Request resolved: #10791

Reviewed By: Yuhta

Differential Revision: D66707660

Pulled By: xiaoxmeng

fbshipit-source-id: f49ee07cf172735eadb8a85e533a2919d0bb48b2
@BryanCutler
Copy link
Contributor

BryanCutler commented Jan 13, 2025 •
edited
Loading

Completed in #10791, can we close this @aditi-pandit ?

@amitkdutta
Copy link
Contributor

@BryanCutler Will be great if we can add casting uuid to varbinary support as well. We are seeing queries fail with CAST( uuid() AS VARBINARY ) SQL. We can do in a separate issue.
CC: @aditi-pandit

@aditi-pandit
Copy link
Collaborator Author

@amitkdutta : Had created issue for CAST(uuid() AS VARBINARY) with the rest of the UUID testing at the time #10585. Have pinged the engineer who wanted to work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BryanCutler BryanCutler

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@BryanCutler @mbasmanova @amitkdutta @aditi-pandit