Enum type added to the types metadata table #5290

LindsayWray · 2022-11-10T21:10:25Z

This pr addresses issue #5174 and offers an alternative solution.

@Mause suggested to output an additional field in the underlying table (duckdb_types) instead, which the pg_... view consumes to provide a postgres compatible interface and therefore compatible with SQLAlchemy. So I’ve added the enum type to duckdb_types.

14 nov. additions:
The pg_enum view now shows the metadata of enum types similar to Postgres. To get the enum data for this view, I have added an extra column to the duckdb_types table containing the possible values of each enum type.
Some small changes in the format_pg_type function and pg_type view were also required to make this functionality resemble the Postgres results as much as possible.

Mytherin

Thanks for the PR! Looks good. Some comments:

test/sql/keywords/keywords_in_expressions.test

test/sql/pg_catalog/sqlalchemy.test

pdet

Hey Lindsay, many thanks for the PR, it looks great!

Out of curiosity, I see that 'ENUM' becomes an invalid name for an enum, is it only on caps, or 'enum'/'EnUm' are also now invalid?

(I don't even remember if we are case sensitive for enum types although I implemented that (: )

LindsayWray · 2022-11-11T11:39:01Z

Hey Lindsay, many thanks for the PR, it looks great!

Out of curiosity, I see that 'ENUM' becomes an invalid name for an enum, is it only on caps, or 'enum'/'EnUm' are also now invalid?

(I don't even remember if we are case sensitive for enum types although I implemented that (: )

it's case insensitive, enum has been added to the catalog and is now like a reserved typename

Mytherin

Thanks for the updates! Looks great.

cpcloud · 2022-11-15T18:43:03Z

cc @saulpw, looks like we can properly support this in ibis now!

Mause · 2022-11-16T05:40:57Z

Looking forward to testing this 😁

Mause · 2022-11-18T05:04:19Z

Doesn't look like this is working as expected quite just yet, I'm looking into it

Mause · 2022-11-18T05:20:48Z

Looks like duckdb_columns() might not properly handle user defined types?

duckdb/src/function/table/system/duckdb_columns.cpp

Line 211 in 9cfe9c4

output.SetValue(10, index, Value::BIGINT(int(type.id())));

vs

duckdb/src/function/table/system/duckdb_types.cpp

Lines 85 to 98 in 9cfe9c4

    
           int64_t oid; 
        
           if (type_entry->internal) { 
        
           	oid = int64_t(type.id()); 
        
           } else { 
        
           	oid = type_entry->oid; 
        
           } 
        
           Value oid_val; 
        
           if (data.oids.find(oid) == data.oids.end()) { 
        
           	data.oids.insert(oid); 
        
           	oid_val = Value::BIGINT(oid); 
        
           } else { 
        
           	oid_val = Value(); 
        
           } 
        
           output.SetValue(2, count, oid_val);

LindsayWray · 2022-11-21T09:46:49Z

Looks like duckdb_columns() might not properly handle user defined types?

I suggest to make this into a separate issue, because I suspect more of these system tables will require similar additions to fully support enums.

Mause · 2022-11-21T10:02:02Z

Looks like duckdb_columns() might not properly handle user defined types?

I suggest to make this into a separate issue, because I suspect more of these system tables will require similar additions to fully support enums.

Sounds likely yes

Rewrite the pattern LIST(x ORDER BY x <ordering>) as LIST_SORT(LIST(x), <ordering>). This reduces the included benchmark run time from 23.5s to 4.32s.

Add missing include for tidy.

Correct benchmark description.

Add more strings to EnumSerializer so LIST_SORT can use it for parsing.

Issue #5290: Rewrite ordered LIST

Lindsay Wray and others added 4 commits November 9, 2022 11:13

git switch trouble

3d66142

ENUM added to duckdb_types()

c360a17

Merge branch 'master' into enum_type_not_by_name

6ee3a6e

rm initial setup

b3c66f0

LindsayWray requested a review from pdet November 11, 2022 06:15

Mytherin reviewed Nov 11, 2022

View reviewed changes

test/sql/keywords/keywords_in_expressions.test Show resolved Hide resolved

test/sql/pg_catalog/sqlalchemy.test Show resolved Hide resolved

pdet approved these changes Nov 11, 2022

View reviewed changes

LindsayWray added the Merge After Feature Freeze label Nov 14, 2022

pg_enum displays metadata

d431cb2

Mytherin changed the base branch from master to feature November 14, 2022 17:24

Mytherin approved these changes Nov 15, 2022

View reviewed changes

Mytherin merged commit 286b243 into duckdb:feature Nov 15, 2022

Mause mentioned this pull request Nov 16, 2022

Ensure enums work correctly Mause/duckdb_engine#449

Open

Mause mentioned this pull request Nov 16, 2022

half-supported ENUM type in DuckDB raises unrelated error message from re.sub sqlalchemy/sqlalchemy#8748

Closed

LindsayWray deleted the enum_type_not_by_name branch November 28, 2022 09:22

hawkfish pushed a commit to hawkfish/duckdb that referenced this pull request Mar 15, 2023

Issue duckdb#5290: Rewrite ordered LIST

5ccbdd1

Rewrite the pattern LIST(x ORDER BY x <ordering>) as LIST_SORT(LIST(x), <ordering>). This reduces the included benchmark run time from 23.5s to 4.32s.

hawkfish pushed a commit to hawkfish/duckdb that referenced this pull request Mar 15, 2023

Issue duckdb#5290: Rewrite ordered LIST

d3f357e

Add missing include for tidy.

hawkfish pushed a commit to hawkfish/duckdb that referenced this pull request Mar 20, 2023

Issue duckdb#5290: Rewrite ordered LIST

eb5b676

Correct benchmark description.

hawkfish pushed a commit to hawkfish/duckdb that referenced this pull request Mar 20, 2023

Issue duckdb#5290: LIST_SORT EnumSerializer

c4144b4

Add more strings to EnumSerializer so LIST_SORT can use it for parsing.

Mytherin added a commit that referenced this pull request Mar 24, 2023

Merge pull request #6741 from hawkfish/sorted-agg

b62a760

Issue #5290: Rewrite ordered LIST

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enum type added to the types metadata table #5290

Enum type added to the types metadata table #5290

LindsayWray commented Nov 10, 2022 •

edited

Loading

Uh oh!

Mytherin left a comment

Uh oh!

Uh oh!

Uh oh!

pdet left a comment •

edited

Loading

Uh oh!

LindsayWray commented Nov 11, 2022

Uh oh!

Mytherin left a comment

Uh oh!

cpcloud commented Nov 15, 2022

Uh oh!

Mause commented Nov 16, 2022

Uh oh!

Mause commented Nov 18, 2022

Uh oh!

Mause commented Nov 18, 2022

Uh oh!

LindsayWray commented Nov 21, 2022

Uh oh!

Mause commented Nov 21, 2022

Uh oh!

Uh oh!

Enum type added to the types metadata table #5290

Enum type added to the types metadata table #5290

Conversation

LindsayWray commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mytherin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pdet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LindsayWray commented Nov 11, 2022

Uh oh!

Mytherin left a comment

Choose a reason for hiding this comment

Uh oh!

cpcloud commented Nov 15, 2022

Uh oh!

Mause commented Nov 16, 2022

Uh oh!

Mause commented Nov 18, 2022

Uh oh!

Mause commented Nov 18, 2022

Uh oh!

LindsayWray commented Nov 21, 2022

Uh oh!

Mause commented Nov 21, 2022

Uh oh!

Uh oh!

LindsayWray commented Nov 10, 2022 •

edited

Loading

pdet left a comment •

edited

Loading