Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE TABLE t_mixed (id INT, complex_val COMPLEX, complex2_val COMPLEX2, int_val INT);
INSERT INTO t_mixed VALUES (1, '(1.0,1.0)', '(2.0,2.0)', 100);
UPDATE t_mixed SET complex2_val = complex_val WHERE id = 1;
ERROR HY000: Cannot implicitly cast from vsql_complex.COMPLEX to vsql_complex.COMPLEX2 for column 'complex2_val' at row 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test should probably be for tvector, since this should be failing even before this change

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added vector test

DROP TABLE t_mixed;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
INSTALL EXTENSION vsql_tvector;
CREATE TABLE t_mixed (id INT, vec2 TVECTOR(2), vec3 TVECTOR(3), int_val INT);
INSERT INTO t_mixed VALUES (1, '[1.0,1.0]', '[2.0,2.0,2.0]', 100);
UPDATE t_mixed SET vec3 = vec2 WHERE id = 1;
ERROR HY000: Cannot implicitly cast from vsql_tvector.TVECTOR(2) to vsql_tvector.TVECTOR(3) for column 'vec3' at row 1
DROP TABLE t_mixed;
UNINSTALL EXTENSION vsql_tvector;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Test INSERT with COMPLEX value into COMPLEX2 field

--source include/villagesql/install_complex_extension.inc

CREATE TABLE t_mixed (id INT, complex_val COMPLEX, complex2_val COMPLEX2, int_val INT);

INSERT INTO t_mixed VALUES (1, '(1.0,1.0)', '(2.0,2.0)', 100);
--error ER_VILLAGESQL_GENERIC_ERROR
UPDATE t_mixed SET complex2_val = complex_val WHERE id = 1;

DROP TABLE t_mixed;

--source include/villagesql/uninstall_complex_extension.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Test INSERT with TVECTOR(2) value into TVECTOR(3) field

INSTALL EXTENSION vsql_tvector;

CREATE TABLE t_mixed (id INT, vec2 TVECTOR(2), vec3 TVECTOR(3), int_val INT);

INSERT INTO t_mixed VALUES (1, '[1.0,1.0]', '[2.0,2.0,2.0]', 100);
--error ER_VILLAGESQL_GENERIC_ERROR
UPDATE t_mixed SET vec3 = vec2 WHERE id = 1;

DROP TABLE t_mixed;

UNINSTALL EXTENSION vsql_tvector;
19 changes: 18 additions & 1 deletion villagesql/schema/descriptor/type_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@
namespace villagesql {

void TypeContext::resolve_cached_values() {
// Build qualified_name_ once: "ext.type" or "ext.type(v1,v2,...)"
// TODO(villagesql): This needs to be updated to support both TYPE(N) and
// TYPE('k1=v1,k2=v2,...') syntax, and to preserve parameter order as defined
// by the extension (currently alphabetical due to std::map). This will be
// revisited when SHOW CREATE TABLE support is added.
qualified_name_ = descriptor_->qualified_base_name();
if (!key_.parameters().empty()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't match what we need for SHOW CREATE TABLE (which is sitting in a branch of mine waiting to rebase on Mike's changes). In particular, we support TYPE(N) and TYPE('k1=v1,k2=v2,...').

I am fine if you want this as a placeholder for now, but I ask you put a TODO here so that I remember to update it when I make my next changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

qualified_name_ += "(";
const char *delim = "";
for (const auto &[k, v] : key_.parameters().params()) {
qualified_name_ += delim;
qualified_name_ += v;
delim = ",";
}
qualified_name_ += ")";
}

if (!key_.parameters().empty() && descriptor_->resolve_params() != nullptr) {
// Variable-length type with parameters: resolve via callback
const auto &param_map = key_.parameters().params();
Expand All @@ -49,7 +66,7 @@ void TypeContext::resolve_cached_values() {
LogVSQL(WARNING_LEVEL,
"resolve_params failed for type '%s' (params: %s): %s. "
"Falling back to descriptor defaults.",
descriptor_->qualified_name().c_str(),
descriptor_->qualified_base_name().c_str(),
key_.parameters().str().c_str(), error_msg);
persisted_length_ = descriptor_->persisted_length();
max_decode_buffer_length_ = descriptor_->max_decode_buffer_length();
Expand Down
7 changes: 5 additions & 2 deletions villagesql/schema/descriptor/type_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ class TypeContext {
return descriptor_->extension_version();
}
const std::string &type_name() const { return descriptor_->type_name(); }
std::string qualified_name() const { return descriptor_->qualified_name(); }
// Returns "extension_name.type_name" or "extension_name.type_name(v1,v2,...)"
// when parameters are present (e.g. "vsql_tvector.TVECTOR(3)").
const std::string &qualified_name() const { return qualified_name_; }

// Storage characteristics for this type instantiation.
// For fixed-length types, these are copied from the TypeDescriptor.
Expand All @@ -246,7 +248,8 @@ class TypeContext {
// Key for this TypeContext (used by ExtensionObjectMap)
TypeContextKey key_;

// Cached storage characteristics (computed eagerly in constructor)
// Cached values (computed eagerly in resolve_cached_values())
std::string qualified_name_;
int64_t persisted_length_{0};
int64_t max_decode_buffer_length_{0};
};
Expand Down
6 changes: 4 additions & 2 deletions villagesql/schema/descriptor/type_descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,10 @@ class TypeDescriptor {
return key_.extension_version();
}

// Returns the fully qualified type name: "extension_name.type_name"
std::string qualified_name() const {
// Returns the qualified base name: "extension_name.type_name"
// Does not include parameters. Use TypeContext::qualified_name() for the
// full name including parameters (e.g. "vsql_tvector.TVECTOR(3)").
std::string qualified_base_name() const {
return extension_name() + "." + type_name();
}

Expand Down
36 changes: 18 additions & 18 deletions villagesql/types/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,9 @@ int CustomMemCompare(const Item *item, const uchar *data1, size_t len1,
}

bool AreTypesCompatible(const TypeContext &tc1, const TypeContext &tc2) {
// Types are compatible if they have identical type name, extension, and
// version
return tc1.type_name() == tc2.type_name() &&
tc1.extension_name() == tc2.extension_name() &&
tc1.extension_version() == tc2.extension_version();
// Types are compatible if they have the same key (type name, extension,
// version, and parameters). This ensures e.g. TVECTOR(3) != TVECTOR(4).
return tc1.key() == tc2.key();
}

bool MaybeValidateUnionTypeCompatibility(Item *accumulator, Item *item) {
Expand Down Expand Up @@ -536,15 +534,15 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
assert(field->has_type_context());

// Check if the item can be stored in the custom field
bool can_store = CanStoreInCustomField(item, field);
if (can_store) {
if (CanStoreInCustomField(item, field)) {
return false; // Success
}

// Validation failed - generate appropriate error message
// Use val_external_str() to get a human-readable value: for custom type items
// this decodes the binary representation; for others it behaves like val_str.
String str_value;
// Need to cast away const to call val_str (which is not const)
String *item_str = const_cast<Item *>(item)->val_str(&str_value);
String *item_str = const_cast<Item *>(item)->val_external_str(&str_value);
const char *value_str = item_str ? item_str->c_ptr_safe() : "<null>";
const THD *thd = current_thd;
const Diagnostics_area *da = thd->get_stmt_da();
Expand Down Expand Up @@ -573,6 +571,12 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
"explicit conversion for column '%s' at row %ld",
MYF(0), field->get_type_context()->type_name().c_str(),
field->field_name, da->current_row_for_condition());
} else if (item->has_type_context()) {
villagesql_error(
"Cannot implicitly cast from %s to %s for column '%s' at row %ld",
MYF(0), item->get_type_context()->qualified_name().c_str(),
field->get_type_context()->qualified_name().c_str(), field->field_name,
da->current_row_for_condition());
} else {
// Generic error for other cases (invalid format, etc.)
villagesql_error("Incorrect %s value: '%s' for column '%s' at row %ld",
Expand All @@ -587,9 +591,10 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
bool TryCopyCustomTypeField(const Field *from, Field *to) {
assert(from->has_type_context());

// If target doesn't have a custom type, this is an incompatible conversion.
if (!to->has_type_context()) {
// TODO(villagesql-performance): evaluate something more performant
// If target doesn't have a custom type, or custom types do not match,
// this is an incompatible conversion.
if (!to->has_type_context() ||
!AreTypesCompatible(*from->get_type_context(), *to->get_type_context())) {
StringBuffer<MAX_FIELD_WIDTH> result(from->charset());
result.length(0U);
from->val_external_str(&result);
Expand All @@ -601,12 +606,7 @@ bool TryCopyCustomTypeField(const Field *from, Field *to) {
MYF(0), from->get_type_context()->type_name().c_str(),
result.c_ptr_safe(), to->field_name,
thd->get_stmt_da()->current_row_for_condition());
return false; // Error generated, don't fall through
}

// Check if they're the same custom type
if (from->get_type_context() != to->get_type_context()) {
return true;
return false;
}

// Both fields have the same custom type. Copy binary data directly.
Expand Down