Fix system table comparisons

Plus missed a few places for schema comparisons

Author: villagestevers

villagesql/schema/util.cc

@@ -19,9 +19,6 @@
 
 namespace villagesql {
 
-// TODO(villagesql-beta): Consolidate to one set of functions for checking
-// villagesql schema name with proper charset. Also check all of the sites
-// that use the constants from SchemaManager to make sure they use this.
 bool is_villagesql_system_table(const TABLE *table) {
   if (!table || !table->s) return false;
   return is_villagesql_schema(table->s->db.str);

villagesql/schema/util.h

@@ -24,20 +24,32 @@ struct TABLE;
 
 namespace villagesql {
 
+// Case-insensitive comparison for system table/schema identifiers, using
+// the system charset. This is how MySQL compares system object names.
+inline bool system_name_eq(const char *a, const char *b) {
+  return my_strcasecmp(system_charset_info, a, b) == 0;
+}
+
 // Check if a database name is the 'villagesql' schema. db_name must not be
 // nullptr.
 inline bool is_villagesql_schema(const char *db_name) {
-  return (my_strcasecmp(system_charset_info,
-                        SchemaManager::VILLAGESQL_SCHEMA_NAME, db_name) == 0);
+  return system_name_eq(SchemaManager::VILLAGESQL_SCHEMA_NAME, db_name);
 }
 
 // Like the above but also considered true if this is in a mysql system schema.
 inline bool is_system_schema(const char *db_name) {
-  return (my_strcasecmp(system_charset_info, "mysql", db_name) == 0 ||
-          my_strcasecmp(system_charset_info, "sys", db_name) == 0 ||
+  return (system_name_eq("mysql", db_name) || system_name_eq("sys", db_name) ||
           is_villagesql_schema(db_name));
 }
 
+// Check if a schema and table name match a VillageSQL system table.
+inline bool is_villagesql_system_table(const char *schema_name,
+                                       const char *table_name,
+                                       const char *expected_table_name) {
+  return is_villagesql_schema(schema_name) &&
+         system_name_eq(table_name, expected_table_name);
+}
+
 // Check if a TABLE is in the 'villagesql' schema.
 // All tables in the villagesql schema are system tables that can be accessed
 // with no_read_locking from INFORMATION_SCHEMA views.

villagesql/schema/victionary_client.h

@@ -48,6 +48,7 @@
 #include "villagesql/schema/systable/custom_columns.h"
 #include "villagesql/schema/systable/extensions.h"
 #include "villagesql/schema/systable/properties.h"
+#include "villagesql/schema/util.h"
 
 // Forward declaration for friend class
 namespace villagesql_unittest {
@@ -844,8 +845,8 @@ inline TABLE *find_open_table(THD *thd, const char *schema_name,
 
   for (TABLE *table = thd->open_tables; table; table = table->next) {
     Table_ref *table_list = table->pos_in_table_list;
-    if (table_list && strcmp(table_list->db, schema_name) == 0 &&
-        strcmp(table_list->table_name, table_name) == 0) {
+    if (table_list && system_name_eq(table_list->db, schema_name) &&
+        system_name_eq(table_list->table_name, table_name)) {
       return table;
     }
   }

villagesql/sql/metadata_modifier.cc

@@ -74,8 +74,7 @@ bool Metadata_modifier::add_columns(THD *thd [[maybe_unused]],
   const char *table_name = db_table.second;
 
   // Skip special databases
-  if (strcmp(db_name, "mysql") == 0 || strcmp(db_name, "sys") == 0 ||
-      strcmp(db_name, SchemaManager::VILLAGESQL_SCHEMA_NAME) == 0) {
+  if (is_system_schema(db_name)) {
     return false;
   }
 
@@ -131,8 +130,7 @@ bool Metadata_modifier::remove_columns(THD *thd [[maybe_unused]],
   const char *table_name = db_table.second;
 
   // Skip special databases
-  if (strcmp(db_name, "mysql") == 0 || strcmp(db_name, "sys") == 0 ||
-      strcmp(db_name, SchemaManager::VILLAGESQL_SCHEMA_NAME) == 0) {
+  if (is_system_schema(db_name)) {
     return false;
   }
 
@@ -229,8 +227,7 @@ bool Metadata_modifier::alter_columns(THD *thd [[maybe_unused]],
   const char *table_name = db_table.second;
 
   // Skip special databases
-  if (strcmp(db_name, "mysql") == 0 || strcmp(db_name, "sys") == 0 ||
-      strcmp(db_name, SchemaManager::VILLAGESQL_SCHEMA_NAME) == 0) {
+  if (is_system_schema(db_name)) {
     return false;
   }
 
@@ -721,10 +718,8 @@ bool Metadata_modifier::store(THD *thd) {
     // just need to find it in query_tables and lock it.
     Table_ref *columns_table = nullptr;
     for (Table_ref *tl = thd->lex->query_tables; tl; tl = tl->next_global) {
-      // TODO(villagesql-beta): table names should be using system charset
-      // comparison
-      if (villagesql::is_villagesql_schema(tl->db) &&
-          strcmp(tl->table_name, SchemaManager::COLUMNS_TABLE_NAME) == 0) {
+      if (villagesql::is_villagesql_system_table(
+              tl->db, tl->table_name, SchemaManager::COLUMNS_TABLE_NAME)) {
         columns_table = tl;
         break;
       }