Skip to content

Add UUID type and operators#755

Merged
electrum merged 1 commit intotrinodb:masterfrom
Praveen2112:uuid_type
May 17, 2019
Merged

Add UUID type and operators#755
electrum merged 1 commit intotrinodb:masterfrom
Praveen2112:uuid_type

Conversation

@Praveen2112
Copy link
Member

No description provided.

@cla-bot cla-bot bot added the cla-signed label May 12, 2019
Copy link
Member

@pgagnon pgagnon left a comment

Choose a reason for hiding this comment

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

@Praveen2112 There is an extra file in your commit (presto-main/hs_err_pid39366.log) that I don't think you intended to add.

@Praveen2112
Copy link
Member Author

@pgagnon Thanks for pointing it out. Have removed it.

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

Did a quick review and it looks good. Please add this to the types documentation, like IP address.

@dain
Copy link
Member

dain commented May 12, 2019

Copy link
Member

@martint martint May 12, 2019

Choose a reason for hiding this comment

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

For acronyms, we capitalize the first letter and use lower case for the rest when used in class or method names (URL -> Url, HTTP -> Http). So, in this case: UuidOperators

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to the above format

@electrum
Copy link
Member

@dain I’m didn’t know that existed, and it’s not documented. We could replace it with a function that returns the new type. PostgreSQL has more specifically named functions like uuid_generate_v4, but given that random is the most common type, it seems fine to use uuid over a more specific name like uuid_random, as that is the most common type (and likely the only one we will ever need to support).

@Praveen2112 Praveen2112 force-pushed the uuid_type branch 2 times, most recently from af2c898 to 9f39502 Compare May 15, 2019 08:01
@Praveen2112
Copy link
Member Author

@electrum Have handled all the commits. Please let me know if there are any more changes.

@electrum electrum merged commit 272a4d6 into trinodb:master May 17, 2019
@electrum
Copy link
Member

Merged, thanks!

@Praveen2112 Praveen2112 deleted the uuid_type branch May 18, 2019 06:43
@electrum electrum mentioned this pull request May 29, 2019
6 tasks
@electrum electrum added this to the 312 milestone May 29, 2019
v-jizhang added a commit to v-jizhang/presto that referenced this pull request Aug 6, 2021
Cherry-pick of trinodb/trino#755.

Co-authored-by: praveenkrishna <praveenkrishna@tutanota.com>
rschlussel pushed a commit to prestodb/presto that referenced this pull request Aug 10, 2021
Cherry-pick of trinodb/trino#755.

Co-authored-by: praveenkrishna <praveenkrishna@tutanota.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants