Skip to content

Commit

Permalink
CREATE TABLE now supports columns with ENUM[] types. (duckdb#14102)
Browse files Browse the repository at this point in the history 
This PR fixes duckdb#14099 

I also noticed that `catalog.schema.user_type` and `schema.user_type`
don't support this, and attempted to fix this here directly, but the
issue appears to be at the parser level so for now I've just added a
test so we're aware of this limitation.

I created another method to deal with all of the base_type logic,
letting the arrayBounds be processed in the outer layer, removing a
bunch of if/else chains and cleaning up the code because of that.
  • Loading branch information
@Mytherin
Mytherin authored Sep 25, 2024
2 parents dfdd09f + f405fb2 commit 45559f5
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 87 deletions.
2 changes: 2 additions & 0 deletions src/include/duckdb/parser/transformer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ class Transformer {
//! Transform a range var into a (schema) qualified name
QualifiedName TransformQualifiedName(duckdb_libpgquery::PGRangeVar &root);

//! Transform a Postgres TypeName string into a LogicalType (non-LIST types)
LogicalType TransformTypeNameInternal(duckdb_libpgquery::PGTypeName &name);
//! Transform a Postgres TypeName string into a LogicalType
LogicalType TransformTypeName(duckdb_libpgquery::PGTypeName &name);

Expand Down
176 changes: 89 additions & 87 deletions src/parser/transform/helpers/transform_typename.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,7 @@ vector<Value> Transformer::TransformTypeModifiers(duckdb_libpgquery::PGTypeName
return type_mods;
}

LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_name) {
if (type_name.type != duckdb_libpgquery::T_PGTypeName) {
throw ParserException("Expected a type");
}
auto stack_checker = StackCheck();

LogicalType Transformer::TransformTypeNameInternal(duckdb_libpgquery::PGTypeName &type_name) {
if (type_name.names->length > 1) {
// qualified typename
vector<string> names;
Expand All @@ -85,24 +80,27 @@ LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_n
}
vector<Value> type_mods = TransformTypeModifiers(type_name);
switch (type_name.names->length) {
case 2:
case 2: {
return LogicalType::USER(INVALID_CATALOG, std::move(names[0]), std::move(names[1]), std::move(type_mods));
case 3:
}
case 3: {
return LogicalType::USER(std::move(names[0]), std::move(names[1]), std::move(names[2]),
std::move(type_mods));
}
default:
throw ParserException(
"Too many qualifications for type name - expected [catalog.schema.name] or [schema.name]");
}
}

auto name = PGPointerCast<duckdb_libpgquery::PGValue>(type_name.names->tail->data.ptr_value)->val.str;
// transform it to the SQL type
LogicalTypeId base_type = TransformStringToLogicalTypeId(name);

LogicalType result_type;
if (base_type == LogicalTypeId::LIST) {
throw ParserException("LIST is not valid as a stand-alone type");
} else if (base_type == LogicalTypeId::ENUM) {
}
if (base_type == LogicalTypeId::ENUM) {
if (!type_name.typmods || type_name.typmods->length == 0) {
throw ParserException("Enum needs a set of entries");
}
Expand All @@ -118,7 +116,8 @@ LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_n
string_data[pos++] = StringVector::AddString(enum_vector, constant_value->val.val.str);
}
return LogicalType::ENUM(enum_vector, NumericCast<idx_t>(type_name.typmods->length));
} else if (base_type == LogicalTypeId::STRUCT) {
}
if (base_type == LogicalTypeId::STRUCT) {
if (!type_name.typmods || type_name.typmods->length == 0) {
throw ParserException("Struct needs a name and entries");
}
Expand Down Expand Up @@ -148,9 +147,9 @@ LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_n
children.push_back(make_pair(entry_name, entry_type));
}
D_ASSERT(!children.empty());
result_type = LogicalType::STRUCT(children);

} else if (base_type == LogicalTypeId::MAP) {
return LogicalType::STRUCT(children);
}
if (base_type == LogicalTypeId::MAP) {
if (!type_name.typmods || type_name.typmods->length != 2) {
throw ParserException("Map type needs exactly two entries, key and value type");
}
Expand All @@ -159,8 +158,9 @@ LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_n
auto value_type =
TransformTypeName(*PGPointerCast<duckdb_libpgquery::PGTypeName>(type_name.typmods->tail->data.ptr_value));

result_type = LogicalType::MAP(std::move(key_type), std::move(value_type));
} else if (base_type == LogicalTypeId::UNION) {
return LogicalType::MAP(std::move(key_type), std::move(value_type));
}
if (base_type == LogicalTypeId::UNION) {
if (!type_name.typmods || type_name.typmods->length == 0) {
throw ParserException("Union type needs at least one member");
}
Expand Down Expand Up @@ -195,81 +195,83 @@ LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_n
children.push_back(make_pair(entry_name, entry_type));
}
D_ASSERT(!children.empty());
result_type = LogicalType::UNION(std::move(children));
} else if (base_type == LogicalTypeId::USER) {
return LogicalType::UNION(std::move(children));
}
if (base_type == LogicalTypeId::USER) {
string user_type_name {name};
vector<Value> type_mods = TransformTypeModifiers(type_name);
result_type = LogicalType::USER(user_type_name, type_mods);
} else {
SizeModifiers modifiers = GetSizeModifiers(type_name, base_type);
switch (base_type) {
case LogicalTypeId::VARCHAR:
if (modifiers.count > 1) {
throw ParserException("VARCHAR only supports a single modifier");
}
// FIXME: create CHECK constraint based on varchar width
modifiers.width = 0;
result_type = LogicalType::VARCHAR;
break;
case LogicalTypeId::DECIMAL:
if (modifiers.count > 2) {
throw ParserException("DECIMAL only supports a maximum of two modifiers");
}
if (modifiers.count == 1) {
// only width is provided: set scale to 0
modifiers.scale = 0;
}
if (modifiers.width <= 0 || modifiers.width > Decimal::MAX_WIDTH_DECIMAL) {
throw ParserException("Width must be between 1 and %d!", (int)Decimal::MAX_WIDTH_DECIMAL);
}
if (modifiers.scale > modifiers.width) {
throw ParserException("Scale cannot be bigger than width");
}
result_type =
LogicalType::DECIMAL(NumericCast<uint8_t>(modifiers.width), NumericCast<uint8_t>(modifiers.scale));
break;
case LogicalTypeId::INTERVAL:
if (modifiers.count > 1) {
throw ParserException("INTERVAL only supports a single modifier");
}
modifiers.width = 0;
result_type = LogicalType::INTERVAL;
break;
case LogicalTypeId::BIT:
if (!modifiers.width && type_name.typmods) {
throw ParserException("Type %s does not support any modifiers!", LogicalType(base_type).ToString());
}
result_type = LogicalType(base_type);
break;
case LogicalTypeId::TIMESTAMP:
if (modifiers.count == 0) {
result_type = LogicalType::TIMESTAMP;
} else {
if (modifiers.count > 1) {
throw ParserException("TIMESTAMP only supports a single modifier");
}
if (modifiers.width > 10) {
throw ParserException("TIMESTAMP only supports until nano-second precision (9)");
}
if (modifiers.width == 0) {
result_type = LogicalType::TIMESTAMP_S;
} else if (modifiers.width <= 3) {
result_type = LogicalType::TIMESTAMP_MS;
} else if (modifiers.width <= 6) {
result_type = LogicalType::TIMESTAMP;
} else {
result_type = LogicalType::TIMESTAMP_NS;
}
}
break;
default:
if (modifiers.count > 0) {
throw ParserException("Type %s does not support any modifiers!", LogicalType(base_type).ToString());
}
result_type = LogicalType(base_type);
break;
return LogicalType::USER(user_type_name, type_mods);
}

SizeModifiers modifiers = GetSizeModifiers(type_name, base_type);
switch (base_type) {
case LogicalTypeId::VARCHAR:
if (modifiers.count > 1) {
throw ParserException("VARCHAR only supports a single modifier");
}
// FIXME: create CHECK constraint based on varchar width
modifiers.width = 0;
return LogicalType::VARCHAR;
case LogicalTypeId::DECIMAL:
if (modifiers.count > 2) {
throw ParserException("DECIMAL only supports a maximum of two modifiers");
}
if (modifiers.count == 1) {
// only width is provided: set scale to 0
modifiers.scale = 0;
}
if (modifiers.width <= 0 || modifiers.width > Decimal::MAX_WIDTH_DECIMAL) {
throw ParserException("Width must be between 1 and %d!", (int)Decimal::MAX_WIDTH_DECIMAL);
}
if (modifiers.scale > modifiers.width) {
throw ParserException("Scale cannot be bigger than width");
}
return LogicalType::DECIMAL(NumericCast<uint8_t>(modifiers.width), NumericCast<uint8_t>(modifiers.scale));
case LogicalTypeId::INTERVAL:
if (modifiers.count > 1) {
throw ParserException("INTERVAL only supports a single modifier");
}
modifiers.width = 0;
return LogicalType::INTERVAL;
case LogicalTypeId::BIT:
if (!modifiers.width && type_name.typmods) {
throw ParserException("Type %s does not support any modifiers!", LogicalType(base_type).ToString());
}
return LogicalType(base_type);
case LogicalTypeId::TIMESTAMP:
if (modifiers.count == 0) {
return LogicalType::TIMESTAMP;
}
if (modifiers.count > 1) {
throw ParserException("TIMESTAMP only supports a single modifier");
}
if (modifiers.width > 10) {
throw ParserException("TIMESTAMP only supports until nano-second precision (9)");
}
if (modifiers.width == 0) {
return LogicalType::TIMESTAMP_S;
}
if (modifiers.width <= 3) {
return LogicalType::TIMESTAMP_MS;
}
if (modifiers.width <= 6) {
return LogicalType::TIMESTAMP;
}
return LogicalType::TIMESTAMP_NS;
default:
if (modifiers.count > 0) {
throw ParserException("Type %s does not support any modifiers!", LogicalType(base_type).ToString());
}
return LogicalType(base_type);
}
}

LogicalType Transformer::TransformTypeName(duckdb_libpgquery::PGTypeName &type_name) {
if (type_name.type != duckdb_libpgquery::T_PGTypeName) {
throw ParserException("Expected a type");
}
auto stack_checker = StackCheck();
auto result_type = TransformTypeNameInternal(type_name);
if (type_name.arrayBounds) {
// array bounds: turn the type into a list
idx_t extra_stack = 0;
Expand Down
46 changes: 46 additions & 0 deletions test/sql/create/create_table_with_arraybounds.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# name: test/sql/create/create_table_with_arraybounds.test
# group: [create]

require noforcestorage

# Create a table with an ENUM[] type
statement ok
create table T (
vis enum ('hide', 'visible')[]
);

query I
select column_type from (describe T);
----
ENUM('hide', 'visible')[]

statement ok
attach ':memory:' as db2;

statement ok
create schema schema2;

statement ok
create schema db2.schema3;

statement ok
create type schema2.foo as VARCHAR;

statement ok
create type db2.schema3.bar as BOOL;

# Create a table with a USER[] type qualified with a schema
statement error
create table B (
vis schema2.foo[]
);
----
syntax error at or near

# Create a table with a USER[] type qualified with a schema and a catalog
statement error
create table B (
vis db2.schema3.bar[]
);
----
syntax error at or near

0 comments on commit 45559f5

Please sign in to comment.