Skip to content

[native] Disable IPAddress type on presto native#23436

Merged
mohsaka merged 1 commit intoprestodb:masterfrom
mohsaka:disable_ipaddress_native
Aug 23, 2024
Merged

[native] Disable IPAddress type on presto native#23436
mohsaka merged 1 commit intoprestodb:masterfrom
mohsaka:disable_ipaddress_native

Conversation

@mohsaka
Copy link
Contributor

@mohsaka mohsaka commented Aug 13, 2024

Description

We have merged in support for IPADDRESS type in velox, however the order by is not supported due to
facebookincubator/velox#10338

As a result, we will block IPAddress type on presto native similar to how timezone with timestamp is disabled via
#23200

Motivation and Context

We do not want correctness failures since IPAddress type has been added.

More context:
facebookincubator/velox#10596 (comment)

Impact

Test Plan

Followed the tests from the Timestamp with timezone disabling.

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... :pr:`12345`
* ... :pr:`12345`

Hive Connector Changes
* ... :pr:`12345`
* ... :pr:`12345`

If release note is NOT required, use:

== NO RELEASE NOTE ==

@mohsaka mohsaka force-pushed the disable_ipaddress_native branch from 7cb8f00 to d0d9e30 Compare August 15, 2024 21:42
@feilong-liu feilong-liu requested a review from amitkdutta August 16, 2024 20:41
@mohsaka mohsaka requested a review from feilong-liu August 16, 2024 23:13
@mohsaka
Copy link
Contributor Author

mohsaka commented Aug 16, 2024

@feilong-liu Really appreciate the thorough review! Pretty sure I got everything fixed up. Please let me know if there were any misunderstandings or other comments. Thanks!

@amitkdutta amitkdutta changed the title [Native] Disable IPAddress type on presto native [native] Disable IPAddress type on presto native Aug 19, 2024
@mohsaka mohsaka requested a review from feilong-liu August 19, 2024 18:20
Copy link
Contributor

@aditi-pandit aditi-pandit left a comment

Choose a reason for hiding this comment

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

Thanks @mohsaka

@mohsaka mohsaka force-pushed the disable_ipaddress_native branch from 77fdd09 to d5d4784 Compare August 23, 2024 03:50
@mohsaka mohsaka merged commit 63a472b into prestodb:master Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants