fix: deny table with empty columns.

project-tsurugi · Jul 7, 2024 · 2f34286 · 2f34286
1 parent ecc5dc6
commit 2f34286
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 3 deletions.
diff --git a/src/mizugaki/analyzer/details/analyze_statement.cpp b/src/mizugaki/analyzer/details/analyze_statement.cpp
@@ -628,6 +628,7 @@ class engine {
 
         auto table_name = normalize_identifier(context_, stmt.name()->last_name());
 
+        // extract columns
         ::takatori::util::reference_vector<::yugawara::storage::column> table_columns {};
         ::tsl::hopscotch_set<std::string_view> saw_columns {};
         table_columns.reserve(stmt.elements().size());
@@ -756,6 +757,13 @@ class engine {
             }
             table_columns.push_back(std::move(prototype));
         }
+        if (table_columns.empty()) {
+            context_.report(
+                    sql_analyzer_code::malformed_syntax,
+                    "table definition must have at least one column",
+                    stmt.region());
+            return {};
+        }
         saw_columns.clear();
 
         auto declaration = std::make_shared<::yugawara::storage::table>(
@@ -919,7 +927,7 @@ class engine {
         query_scope scope {};
         scope.add(build_relation_info(context_, *table, false));
 
-        auto index_keys = build_index_keys(constraint.key(), scope);
+        auto index_keys = build_index_keys(constraint.key(), scope, constraint.region());
         if (index_keys.empty()) {
             return {};
         }
@@ -937,7 +945,8 @@ class engine {
 
     [[nodiscard]] std::vector<::yugawara::storage::index::key> build_index_keys(
             std::vector<ast::common::sort_element> const& source_keys,
-            query_scope const& scope) {
+            query_scope const& scope,
+            ast::node_region region) {
         ::tsl::hopscotch_set<::yugawara::storage::column const*> saw_columns { source_keys.size() };
         auto index_keys = create_vector<::yugawara::storage::index::key>(source_keys.size());
         for (auto&& key : source_keys) {
@@ -990,6 +999,13 @@ class engine {
             saw_columns.insert(binding.get());
             index_keys.emplace_back(*binding, to_sort_direction(key.direction()));
         }
+        if (index_keys.empty()) {
+            context_.report(
+                    sql_analyzer_code::malformed_syntax,
+                    "index with empty key is not supported",
+                    region);
+            return {};
+        }
         return index_keys;
     }
 
@@ -1047,6 +1063,7 @@ class engine {
 
         auto&& table_ptr = table_result->second;
 
+        ast::node_region region;
         std::string name {};
         if (stmt.name()) {
             // FIXME: impl qualified index name
@@ -1058,12 +1075,13 @@ class engine {
                 return {};
             }
             name = normalize_identifier(context_, stmt.name()->last_name());
+            region = stmt.name()->region();
         }
 
         query_scope scope {};
         scope.add(build_relation_info(context_, *table_ptr, false));
 
-        auto index_keys = build_index_keys(stmt.keys(), scope);
+        auto index_keys = build_index_keys(stmt.keys(), scope, region);
         if (index_keys.empty()) {
             return {};
         }

diff --git a/test/mizugaki/analyzer/details/analyze_statement_index_definition_test.cpp b/test/mizugaki/analyzer/details/analyze_statement_index_definition_test.cpp
@@ -159,6 +159,15 @@ TEST_F(analyze_statement_index_definition_test, columns_multiple) {
     }
 }
 
+TEST_F(analyze_statement_index_definition_test, columns_missing) {
+    auto table = install_table("testing_table");
+    invalid(sql_analyzer_code::malformed_syntax, ast::statement::index_definition {
+            id("testing"),
+            id("testing_table"),
+            {},
+    });
+}
+
 TEST_F(analyze_statement_index_definition_test, direction) {
     auto table = install_table("testing_table");
     auto r = analyze_statement(context(), ast::statement::index_definition {

diff --git a/test/mizugaki/analyzer/details/analyze_statement_table_definition_test.cpp b/test/mizugaki/analyzer/details/analyze_statement_table_definition_test.cpp
@@ -139,6 +139,13 @@ TEST_F(analyze_statement_table_definition_test, column_multiple) {
     }
 }
 
+TEST_F(analyze_statement_table_definition_test, column_missing) {
+    invalid(sql_analyzer_code::malformed_syntax, ast::statement::table_definition {
+            id("testing"),
+            {},
+    });
+}
+
 TEST_F(analyze_statement_table_definition_test, column_primary_key) {
     auto r = analyze_statement(context(), ast::statement::table_definition {
             id("testing"),
@@ -447,6 +454,24 @@ TEST_F(analyze_statement_table_definition_test, table_primary_key_multiple_colum
     }));
 }
 
+TEST_F(analyze_statement_table_definition_test, table_primary_key_missing_columns) {
+    invalid(sql_analyzer_code::malformed_syntax, ast::statement::table_definition {
+            id("testing"),
+            {
+                    ast::statement::column_definition {
+                            id("c1"),
+                            ast::type::simple { ast::type::kind::integer },
+                    },
+                    ast::statement::table_constraint_definition {
+                            ast::statement::key_constraint {
+                                    ast::statement::constraint_kind::primary_key,
+                                    {},
+                            },
+                    },
+            },
+    });
+}
+
 TEST_F(analyze_statement_table_definition_test, duplicate_target) {
     install_table("testing"); // conflict
     invalid(sql_analyzer_code::table_already_exists, ast::statement::table_definition {