rebased, refactored and tvector fixes

tomas-villagesql · tomas-villagesql · commit 60666041435a · 2026-02-26T11:43:27.000+01:00
diff --git a/mysql-test/suite/villagesql/alter_table/r/alter_table_change_tvector_column.result b/mysql-test/suite/villagesql/alter_table/r/alter_table_change_tvector_column.result
@@ -0,0 +1,61 @@
+INSTALL EXTENSION vsql_tvector;
+CREATE TABLE t1 (
+id INT PRIMARY KEY,
+vec3 TVECTOR(3)
+);
+INSERT INTO t1 VALUES (1, '[1.0,2.0,3.0]');
+INSERT INTO t1 VALUES (2, '[4.5,5.5,6.5]');
+INSERT INTO t1 VALUES (3, NULL);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id` int NOT NULL,
+  `vec3` vsql_tvector.TVECTOR DEFAULT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
+SELECT * FROM t1 ORDER BY id;
+id	vec3
+1	[1,2,3]
+2	[4.5,5.5,6.5]
+3	NULL
+ALTER TABLE t1 CHANGE vec3 vec4 TVECTOR(4);
+ERROR HY000: Cannot convert between incompatible custom types 'vsql_tvector.TVECTOR(3)' and 'vsql_tvector.TVECTOR(4)'
+ALTER TABLE t1 CHANGE vec3 vec3_2 TVECTOR(3);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id` int NOT NULL,
+  `vec3_2` vsql_tvector.TVECTOR DEFAULT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
+SELECT * FROM t1 ORDER BY id;
+id	vec3_2
+1	[1,2,3]
+2	[4.5,5.5,6.5]
+3	NULL
+ALTER TABLE t1 CHANGE vec3_2 vec3_str VARCHAR(100);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id` int NOT NULL,
+  `vec3_str` varchar(100) DEFAULT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
+SELECT * FROM t1 ORDER BY id;
+id	vec3_str
+1	[1,2,3]
+2	[4.5,5.5,6.5]
+3	NULL
+DROP TABLE t1;
+CREATE TABLE t2 (
+id INT PRIMARY KEY,
+vec2 TVECTOR(2),
+vec3 TVECTOR(3)
+);
+INSERT INTO t2 VALUES (1, '[-1.0,2.5]', '[1.0,2.0,3.0]');
+INSERT INTO t2 VALUES (2, '[4.0,-5.0]', '[4.5,5.5,6.5]');
+INSERT INTO t2 VALUES (3, NULL, NULL);
+SELECT * FROM t2 WHERE vec2 = vec3;
+ERROR HY000: Cannot compare types vsql_tvector.TVECTOR(2) and vsql_tvector.TVECTOR(3) in =
+DROP TABLE t2;
+UNINSTALL EXTENSION vsql_tvector;
diff --git a/mysql-test/suite/villagesql/alter_table/t/alter_table_change_tvector_column.test b/mysql-test/suite/villagesql/alter_table/t/alter_table_change_tvector_column.test
@@ -0,0 +1,49 @@
+# Test ALTER TABLE CHANGE on existing TVECTOR column
+# Tests copy between TVECTOR(N) columns with same and different prefix sizes,
+# and conversion to string.
+
+INSTALL EXTENSION vsql_tvector;
+
+CREATE TABLE t1 (
+    id INT PRIMARY KEY,
+    vec3 TVECTOR(3)
+);
+
+INSERT INTO t1 VALUES (1, '[1.0,2.0,3.0]');
+INSERT INTO t1 VALUES (2, '[4.5,5.5,6.5]');
+INSERT INTO t1 VALUES (3, NULL);
+
+SHOW CREATE TABLE t1;
+SELECT * FROM t1 ORDER BY id;
+
+--error ER_VILLAGESQL_GENERIC_ERROR
+ALTER TABLE t1 CHANGE vec3 vec4 TVECTOR(4);
+
+ALTER TABLE t1 CHANGE vec3 vec3_2 TVECTOR(3);
+
+SHOW CREATE TABLE t1;
+SELECT * FROM t1 ORDER BY id;
+
+ALTER TABLE t1 CHANGE vec3_2 vec3_str VARCHAR(100);
+
+SHOW CREATE TABLE t1;
+SELECT * FROM t1 ORDER BY id;
+
+DROP TABLE t1;
+
+CREATE TABLE t2 (
+    id INT PRIMARY KEY,
+    vec2 TVECTOR(2),
+    vec3 TVECTOR(3)
+);
+
+INSERT INTO t2 VALUES (1, '[-1.0,2.5]', '[1.0,2.0,3.0]');
+INSERT INTO t2 VALUES (2, '[4.0,-5.0]', '[4.5,5.5,6.5]');
+INSERT INTO t2 VALUES (3, NULL, NULL);
+
+--error ER_VILLAGESQL_GENERIC_ERROR
+SELECT * FROM t2 WHERE vec2 = vec3;
+
+DROP TABLE t2;
+
+UNINSTALL EXTENSION vsql_tvector;
diff --git a/sql/field_conv.cc b/sql/field_conv.cc
@@ -324,17 +324,6 @@ static void do_field_string(Copy_field *, const Field *from_field,
   to_field->store(res.ptr(), res.length(), res.charset());
 }
 
-// VillageSQL: Copy from a custom type field to custom type field.
-static void do_field_custom_to_custom(Copy_field *, const Field *from_field,
-                                      Field *to_field) {
-  villagesql::CopyCustomToCustomField(from_field, to_field);
-}
-
-// VillageSQL: Copy from a custom type field to string field.
-static void do_field_custom_to_string(Copy_field *, const Field *from_field,
-                                      Field *to_field) {
-  villagesql::CopyCustomToStringField(from_field, to_field);
-}
 
 static void do_field_enum(Copy_field *copy, const Field *from_field,
                           Field *to_field) {
@@ -578,11 +567,7 @@ Copy_field::Copy_func *Copy_field::get_copy_func() {
 
   // VillageSQL: Route to the appropriate custom type copy function.
   if (m_from_field->has_type_context()) {
-    if (m_to_field->has_type_context()) {
-      return do_field_custom_to_custom;
-    } else {
-      return do_field_custom_to_string;
-    }
+    return villagesql::GetCopyFunc(m_from_field, m_to_field);
   }
 
   const bool compatible_db_low_byte_first =
diff --git a/villagesql/sql/metadata_modifier.cc b/villagesql/sql/metadata_modifier.cc
@@ -300,7 +300,7 @@ bool Metadata_modifier::alter_columns(THD *thd [[maybe_unused]],
           ColumnEntry new_entry(
               ColumnKey(db_name, table_name, alter->m_new_name),
               old_entry_ptr->extension_name, old_entry_ptr->extension_version,
-              old_entry_ptr->type_name);
+              old_entry_ptr->type_name, old_entry_ptr->type_parameters);
 
           to_rename_.emplace_back(new_entry, old_entry_ptr->key());
         }
@@ -361,7 +361,8 @@ bool Metadata_modifier::alter_columns(THD *thd [[maybe_unused]],
         to_add_.emplace_back(ColumnKey(db_name, table_name, field.field_name),
                              field.custom_type_context->extension_name(),
                              field.custom_type_context->extension_version(),
-                             field.custom_type_context->type_name());
+                             field.custom_type_context->type_name(),
+                             field.custom_type_context->parameters().to_json());
       } else if (was_custom_type && is_custom_type) {
         // Changing FROM custom TO custom - use delete-then-insert pattern
         // Note: Apply removals before additions
@@ -371,14 +372,16 @@ bool Metadata_modifier::alter_columns(THD *thd [[maybe_unused]],
         to_add_.emplace_back(ColumnKey(db_name, table_name, field.field_name),
                              field.custom_type_context->extension_name(),
                              field.custom_type_context->extension_version(),
-                             field.custom_type_context->type_name());
+                             field.custom_type_context->type_name(),
+                             field.custom_type_context->parameters().to_json());
       }
     } else if (is_custom_type) {
       // This is ADD COLUMN with custom type - insert entry
       to_add_.emplace_back(ColumnKey(db_name, table_name, field.field_name),
                            field.custom_type_context->extension_name(),
                            field.custom_type_context->extension_version(),
-                           field.custom_type_context->type_name());
+                           field.custom_type_context->type_name(),
+                           field.custom_type_context->parameters().to_json());
     }
   }
   return false;
diff --git a/villagesql/types/util.cc b/villagesql/types/util.cc
@@ -132,8 +132,7 @@ bool MaybeInjectCustomType(THD *thd, TABLE_SHARE &share, Field *field) {
   // persisted_length. A mismatch here would mean InnoDB allocated a different
   // amount of storage than the type expects, which leads to silent data
   // corruption.
-  if (should_assert_if_false(static_cast<int64_t>(field->field_length) ==
-                             tc->persisted_length())) {
+  if (static_cast<int64_t>(field->field_length) != tc->persisted_length()) {
     LogVSQL(ERROR_LEVEL,
             "field_length (%u) != persisted_length (%" PRId64
             ") for column %s in table %s.%s (type %s)",
@@ -604,30 +603,6 @@ bool ValidateAndReportCustomFieldStore(const Item *item, const Field *field) {
   return true;  // Error
 }
 
-bool TryCopyCustomTypeField(const Field *from, Field *to) {
-  assert(from->has_type_context());
-
-  // 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);
-
-    THD *thd = current_thd;
-    villagesql_error(
-        "Type mismatch. Use explicit conversion of %s value '%s' for column "
-        "'%s' at row %ld",
-        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;
-  }
-  CopyCustomToCustomField(from, to);
-  return false;
-}
-
 void CopyCustomToCustomField(const Field *from, Field *to) {
   // Both fields have the same custom type. Copy binary data directly.
   // Handle potential length_bytes differences (VARCHAR(255) vs VARCHAR(65535)).
@@ -654,7 +629,13 @@ void CopyCustomToCustomField(const Field *from, Field *to) {
   memcpy(to_ptr + to_length_bytes, from_data, data_len);
 }
 
-void CopyCustomToStringField(const Field *from, Field *to) {
+static void do_field_custom_to_custom(Copy_field *, const Field *from,
+                                      Field *to) {
+  CopyCustomToCustomField(from, to);
+}
+
+static void do_field_custom_to_string(Copy_field *, const Field *from,
+                                      Field *to) {
   assert(from->has_type_context());
   // Custom → non-custom string: decode to string representation.
   // NULL is handled outside this function
@@ -665,6 +646,38 @@ void CopyCustomToStringField(const Field *from, Field *to) {
   to->store(res.ptr(), res.length(), res.charset());
 }
 
+FieldCopyFunc *GetCopyFunc(const Field *from, const Field *to) {
+  assert(from->has_type_context());
+  if (to->has_type_context()) {
+    return do_field_custom_to_custom;
+  }
+  return do_field_custom_to_string;
+}
+
+bool TryCopyCustomTypeField(const Field *from, Field *to) {
+  assert(from->has_type_context());
+
+  // 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);
+
+    THD *thd = current_thd;
+    villagesql_error(
+        "Type mismatch. Use explicit conversion of %s value '%s' for column "
+        "'%s' at row %ld",
+        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;
+  }
+  CopyCustomToCustomField(from, to);
+  return false;
+}
+
 type_conversion_status TryEncodeStringFieldToCustom(Field *from_field,
                                                     Field *to_field) {
   assert(!from_field->has_type_context());
diff --git a/villagesql/types/util.h b/villagesql/types/util.h
@@ -287,11 +287,13 @@ extern bool ValidateAndReportCustomFieldStore(const Item *item,
 // If to does not have a custom type, generates an error with readable format.
 extern bool TryCopyCustomTypeField(const Field *from, Field *to);
 
-// Binary copy from a custom type field to custom type field.
-extern void CopyCustomToCustomField(const Field *from, Field *to);
+// Function pointer type for field copy operations (matches
+// Copy_field::Copy_func).
+using FieldCopyFunc = void(Copy_field *, const Field *, Field *);
 
-// Copy from a custom type field to string type field.
-extern void CopyCustomToStringField(const Field *from, Field *to);
+// Returns the appropriate copy function for copying from a custom type field.
+// from must have a custom type context.
+extern FieldCopyFunc *GetCopyFunc(const Field *from, const Field *to);
 
 // Encode a string field value and store it in a custom type field.
 // This enables CTEs and subqueries with string values to work with custom
