Skip to content

Commit b79fb2f

Browse files
incompatible custom types block (#60)
1 parent cc006ae commit b79fb2f

8 files changed

Lines changed: 83 additions & 23 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
CREATE TABLE t_mixed (id INT, complex_val COMPLEX, complex2_val COMPLEX2, int_val INT);
2+
INSERT INTO t_mixed VALUES (1, '(1.0,1.0)', '(2.0,2.0)', 100);
3+
UPDATE t_mixed SET complex2_val = complex_val WHERE id = 1;
4+
ERROR HY000: Cannot implicitly cast from vsql_complex.COMPLEX to vsql_complex.COMPLEX2 for column 'complex2_val' at row 1
5+
DROP TABLE t_mixed;
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
INSTALL EXTENSION vsql_tvector;
2+
CREATE TABLE t_mixed (id INT, vec2 TVECTOR(2), vec3 TVECTOR(3), int_val INT);
3+
INSERT INTO t_mixed VALUES (1, '[1.0,1.0]', '[2.0,2.0,2.0]', 100);
4+
UPDATE t_mixed SET vec3 = vec2 WHERE id = 1;
5+
ERROR HY000: Cannot implicitly cast from vsql_tvector.TVECTOR(2) to vsql_tvector.TVECTOR(3) for column 'vec3' at row 1
6+
DROP TABLE t_mixed;
7+
UNINSTALL EXTENSION vsql_tvector;
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Test INSERT with COMPLEX value into COMPLEX2 field
2+
3+
--source include/villagesql/install_complex_extension.inc
4+
5+
CREATE TABLE t_mixed (id INT, complex_val COMPLEX, complex2_val COMPLEX2, int_val INT);
6+
7+
INSERT INTO t_mixed VALUES (1, '(1.0,1.0)', '(2.0,2.0)', 100);
8+
--error ER_VILLAGESQL_GENERIC_ERROR
9+
UPDATE t_mixed SET complex2_val = complex_val WHERE id = 1;
10+
11+
DROP TABLE t_mixed;
12+
13+
--source include/villagesql/uninstall_complex_extension.inc
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Test INSERT with TVECTOR(2) value into TVECTOR(3) field
2+
3+
INSTALL EXTENSION vsql_tvector;
4+
5+
CREATE TABLE t_mixed (id INT, vec2 TVECTOR(2), vec3 TVECTOR(3), int_val INT);
6+
7+
INSERT INTO t_mixed VALUES (1, '[1.0,1.0]', '[2.0,2.0,2.0]', 100);
8+
--error ER_VILLAGESQL_GENERIC_ERROR
9+
UPDATE t_mixed SET vec3 = vec2 WHERE id = 1;
10+
11+
DROP TABLE t_mixed;
12+
13+
UNINSTALL EXTENSION vsql_tvector;

villagesql/schema/descriptor/type_context.cc

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,23 @@
2929
namespace villagesql {
3030

3131
void TypeContext::resolve_cached_values() {
32+
// Build qualified_name_ once: "ext.type" or "ext.type(v1,v2,...)"
33+
// TODO(villagesql): This needs to be updated to support both TYPE(N) and
34+
// TYPE('k1=v1,k2=v2,...') syntax, and to preserve parameter order as defined
35+
// by the extension (currently alphabetical due to std::map). This will be
36+
// revisited when SHOW CREATE TABLE support is added.
37+
qualified_name_ = descriptor_->qualified_base_name();
38+
if (!key_.parameters().empty()) {
39+
qualified_name_ += "(";
40+
const char *delim = "";
41+
for (const auto &[k, v] : key_.parameters().params()) {
42+
qualified_name_ += delim;
43+
qualified_name_ += v;
44+
delim = ",";
45+
}
46+
qualified_name_ += ")";
47+
}
48+
3249
if (!key_.parameters().empty() && descriptor_->resolve_params() != nullptr) {
3350
// Variable-length type with parameters: resolve via callback
3451
const auto &param_map = key_.parameters().params();
@@ -49,7 +66,7 @@ void TypeContext::resolve_cached_values() {
4966
LogVSQL(WARNING_LEVEL,
5067
"resolve_params failed for type '%s' (params: %s): %s. "
5168
"Falling back to descriptor defaults.",
52-
descriptor_->qualified_name().c_str(),
69+
descriptor_->qualified_base_name().c_str(),
5370
key_.parameters().str().c_str(), error_msg);
5471
persisted_length_ = descriptor_->persisted_length();
5572
max_decode_buffer_length_ = descriptor_->max_decode_buffer_length();

villagesql/schema/descriptor/type_context.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,9 @@ class TypeContext {
226226
return descriptor_->extension_version();
227227
}
228228
const std::string &type_name() const { return descriptor_->type_name(); }
229-
std::string qualified_name() const { return descriptor_->qualified_name(); }
229+
// Returns "extension_name.type_name" or "extension_name.type_name(v1,v2,...)"
230+
// when parameters are present (e.g. "vsql_tvector.TVECTOR(3)").
231+
const std::string &qualified_name() const { return qualified_name_; }
230232

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

249-
// Cached storage characteristics (computed eagerly in constructor)
251+
// Cached values (computed eagerly in resolve_cached_values())
252+
std::string qualified_name_;
250253
int64_t persisted_length_{0};
251254
int64_t max_decode_buffer_length_{0};
252255
};

villagesql/schema/descriptor/type_descriptor.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ class TypeDescriptor {
174174
return key_.extension_version();
175175
}
176176

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

villagesql/types/util.cc

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -383,11 +383,9 @@ int CustomMemCompare(const Item *item, const uchar *data1, size_t len1,
383383
}
384384

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

393391
bool MaybeValidateUnionTypeCompatibility(Item *accumulator, Item *item) {
@@ -552,15 +550,15 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
552550
assert(field->has_type_context());
553551

554552
// Check if the item can be stored in the custom field
555-
bool can_store = CanStoreInCustomField(item, field);
556-
if (can_store) {
553+
if (CanStoreInCustomField(item, field)) {
557554
return false; // Success
558555
}
559556

560557
// Validation failed - generate appropriate error message
558+
// Use val_external_str() to get a human-readable value: for custom type items
559+
// this decodes the binary representation; for others it behaves like val_str.
561560
String str_value;
562-
// Need to cast away const to call val_str (which is not const)
563-
String *item_str = const_cast<Item *>(item)->val_str(&str_value);
561+
String *item_str = const_cast<Item *>(item)->val_external_str(&str_value);
564562
const char *value_str = item_str ? item_str->c_ptr_safe() : "<null>";
565563
const THD *thd = current_thd;
566564
const Diagnostics_area *da = thd->get_stmt_da();
@@ -589,6 +587,12 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
589587
"explicit conversion for column '%s' at row %ld",
590588
MYF(0), field->get_type_context()->type_name().c_str(),
591589
field->field_name, da->current_row_for_condition());
590+
} else if (item->has_type_context()) {
591+
villagesql_error(
592+
"Cannot implicitly cast from %s to %s for column '%s' at row %ld",
593+
MYF(0), item->get_type_context()->qualified_name().c_str(),
594+
field->get_type_context()->qualified_name().c_str(), field->field_name,
595+
da->current_row_for_condition());
592596
} else {
593597
// Generic error for other cases (invalid format, etc.)
594598
villagesql_error("Incorrect %s value: '%s' for column '%s' at row %ld",
@@ -603,9 +607,10 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
603607
bool TryCopyCustomTypeField(const Field *from, Field *to) {
604608
assert(from->has_type_context());
605609

606-
// If target doesn't have a custom type, this is an incompatible conversion.
607-
if (!to->has_type_context()) {
608-
// TODO(villagesql-performance): evaluate something more performant
610+
// If target doesn't have a custom type, or custom types do not match,
611+
// this is an incompatible conversion.
612+
if (!to->has_type_context() ||
613+
!AreTypesCompatible(*from->get_type_context(), *to->get_type_context())) {
609614
StringBuffer<MAX_FIELD_WIDTH> result(from->charset());
610615
result.length(0U);
611616
from->val_external_str(&result);
@@ -617,12 +622,7 @@ bool TryCopyCustomTypeField(const Field *from, Field *to) {
617622
MYF(0), from->get_type_context()->type_name().c_str(),
618623
result.c_ptr_safe(), to->field_name,
619624
thd->get_stmt_da()->current_row_for_condition());
620-
return false; // Error generated, don't fall through
621-
}
622-
623-
// Check if they're the same custom type
624-
if (from->get_type_context() != to->get_type_context()) {
625-
return true;
625+
return false;
626626
}
627627

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

0 commit comments

Comments
 (0)