fixed hive view uuid type parsing #24538

adkharat · 2025-02-12T14:43:10Z

Description

Currently, creation of VIEW with UUID on hive catalog throws exception

create or replace view "hive"."dbcert_hive".vuuid as 
SELECT * FROM ( 
 VALUES 
 (cast(0 as integer), null), 
 (cast(1 as integer), UUID '12151fd2-7586-11e9-8f9e-2a86e4085a59') 
) AS t (rum, c1);

This PR fixes UUID type parsing while VIEW creation, selection on Hive

Motivation and Context

Presto is failing to recognize UUID as a valid Hive type, leading to a type resolution failure.

cannot construct instance of com.facebook.presto.hive.HiveType, problem: Error: type expected at the position 0 of uuid'but uuid is found.

Test Plan

Added a test for hive and iceberg where we create the view with uuid column and select from that view.

Release Notes

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

== RELEASE NOTES ==

General Changes
* Fix Hive ``UUID`` type parsing

steveburnett · 2025-02-12T14:57:07Z

Thanks for the release note! Nits of formatting suggested.

== RELEASE NOTES ==

General Changes
* Fix Hive ``UUID`` type parsing.

nmahadevuni · 2025-02-12T17:20:02Z

@adkharat Please add tests for both hive and iceberg.

nmahadevuni · 2025-02-13T12:20:21Z

Don't add tests in new files. You can add tests to existing test file like AbstractTestDistributedQueries.java. Existing hive and iceberg tests derive from this file. So, it will be run for both.

adkharat · 2025-02-14T11:41:39Z

Don't add tests in new files. You can add tests to existing test file like AbstractTestDistributedQueries.java. Existing hive and iceberg tests derive from this file. So, it will be run for both.

@nmahadevuni Added test case in AbstractTestDistributedQueries.java

nmahadevuni · 2025-02-14T13:02:19Z

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java

+        MaterializedResult result = computeActual("SELECT c1 FROM test_hive_view WHERE rum = 1");
+
+        // Verify the result set is not empty
+        assertTrue(result.getMaterializedRows().size() > 0, "Result set is empty");


Can't we compare the UUID value?

No, we cannot directly compare the UUID returned in the result unless casted to string.

As I see, the data maintained in materialized row for uuid is already plain string, so seems we can validate the result as follows:

assertEquals(result.getTypes(), ImmutableList.of(UUID)); assertEquals(result.getOnlyValue(), "12151fd2-7586-11e9-8f9e-2a86e4085a59");

Thanks, it worked.
Changes pushed to compare type and UUID returned in the result.

hantangwangd · 2025-02-14T15:20:36Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveType.java

@@ -227,6 +227,9 @@ public static boolean isSupportedType(TypeInfo typeInfo)
    public static HiveType valueOf(String hiveTypeName)
    {
        requireNonNull(hiveTypeName, "hiveTypeName is null");
+        if (hiveTypeName.equals("uuid")) {


Suggested change

if (hiveTypeName.equals("uuid")) {

if (hiveTypeName.equals(HIVE_UUID.getTypeInfo().getTypeName())) {

Better to use HIVE_UUID.getTypeInfo().getTypeName(), since it is where the value uuid from.

Thanks,

Done. Pushed.

hantangwangd

Thanks for the fix.

auden-woolfson

LGTM

nmahadevuni · 2025-02-14T18:25:51Z

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java

@@ -16,6 +16,7 @@
 import com.facebook.airlift.testing.Assertions;
 import com.facebook.presto.Session;
 import com.facebook.presto.SystemSessionProperties;
+import com.facebook.presto.common.type.UuidType;


import static com.facebook.presto.common.type.UuidType.UUID;

and use UUID in the assert function

Thanks,

Changes pushed.

added view with UUID compare the UUID returned in the result compare with constant uuid imported static constant

adkharat requested a review from a team as a code owner February 12, 2025 14:43

adkharat requested a review from presto-oss February 12, 2025 14:43

prestodb-ci added the from:IBM PR from IBM label Feb 12, 2025

prestodb-ci requested review from a team, auden-woolfson and imsayari404 and removed request for a team February 12, 2025 14:43

adkharat force-pushed the fix_hive_uuid_type_parsing branch from 74e49fa to d5f9256 Compare February 12, 2025 15:47

adkharat requested review from hantangwangd and ZacBlanco as code owners February 13, 2025 09:38

adkharat changed the title ~~fixed hive uuid type parsing~~ fixed hive view uuid type parsing Feb 13, 2025

adkharat force-pushed the fix_hive_uuid_type_parsing branch 2 times, most recently from c9fab35 to f26adef Compare February 13, 2025 11:24

adkharat force-pushed the fix_hive_uuid_type_parsing branch from f26adef to 2f396cc Compare February 14, 2025 05:03

adkharat requested review from jaystarshot, feilong-liu, elharo and ClarenceThreepwood as code owners February 14, 2025 05:03

adkharat marked this pull request as draft February 14, 2025 06:43

adkharat force-pushed the fix_hive_uuid_type_parsing branch 4 times, most recently from ff3bb49 to bfc7065 Compare February 14, 2025 10:42

nmahadevuni reviewed Feb 14, 2025

View reviewed changes

adkharat marked this pull request as ready for review February 14, 2025 13:14

adkharat force-pushed the fix_hive_uuid_type_parsing branch from 52be48e to f87af61 Compare February 14, 2025 14:50

hantangwangd reviewed Feb 14, 2025

View reviewed changes

adkharat force-pushed the fix_hive_uuid_type_parsing branch from 7258103 to 9017291 Compare February 14, 2025 15:34

adkharat requested a review from hantangwangd February 14, 2025 16:38

hantangwangd previously approved these changes Feb 14, 2025

View reviewed changes

jaystarshot previously approved these changes Feb 14, 2025

View reviewed changes

auden-woolfson previously approved these changes Feb 14, 2025

View reviewed changes

nmahadevuni reviewed Feb 14, 2025

View reviewed changes

fixed view uuid type parsing

6d063ad

added view with UUID compare the UUID returned in the result compare with constant uuid imported static constant

adkharat dismissed stale reviews from auden-woolfson, jaystarshot, and hantangwangd via 6d063ad February 14, 2025 22:46

adkharat force-pushed the fix_hive_uuid_type_parsing branch from 9017291 to 6d063ad Compare February 14, 2025 22:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed hive view uuid type parsing #24538

fixed hive view uuid type parsing #24538

adkharat commented Feb 12, 2025 •

edited

Loading

steveburnett commented Feb 12, 2025

nmahadevuni commented Feb 12, 2025

nmahadevuni commented Feb 13, 2025

adkharat commented Feb 14, 2025 •

edited

Loading

nmahadevuni Feb 14, 2025

adkharat Feb 14, 2025

hantangwangd Feb 14, 2025

adkharat Feb 14, 2025

hantangwangd Feb 14, 2025

adkharat Feb 14, 2025

hantangwangd left a comment

auden-woolfson left a comment

nmahadevuni Feb 14, 2025

adkharat Feb 14, 2025

	if (hiveTypeName.equals("uuid")) {
	if (hiveTypeName.equals(HIVE_UUID.getTypeInfo().getTypeName())) {

fixed hive view uuid type parsing #24538

Are you sure you want to change the base?

fixed hive view uuid type parsing #24538

Conversation

adkharat commented Feb 12, 2025 • edited Loading

Description

Motivation and Context

Test Plan

Release Notes

steveburnett commented Feb 12, 2025

nmahadevuni commented Feb 12, 2025

nmahadevuni commented Feb 13, 2025

adkharat commented Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hantangwangd left a comment

Choose a reason for hiding this comment

auden-woolfson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adkharat commented Feb 12, 2025 •

edited

Loading

adkharat commented Feb 14, 2025 •

edited

Loading