Merge pull request #57342 from bharatnc/ncb/better-hint-if-table-does…

…nt-exist

provide a better hint if a table doesn't exist

src/Databases/IDatabase.cpp

@@ -27,12 +27,21 @@ StoragePtr IDatabase::getTable(const String & name, ContextPtr context) const
 {
     if (auto storage = tryGetTable(name, context))
         return storage;
+
     TableNameHints hints(this->shared_from_this(), context);
-    std::vector<String> names = hints.getHints(name);
-    if (names.empty())
+    /// hint is a pair which holds a single database_name and table_name suggestion for the given table name.
+    auto hint = hints.getHintForTable(name);
+
+    if (hint.first.empty())
         throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} does not exist", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
     else
-        throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} does not exist. Maybe you meant {}?", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name), backQuoteIfNeed(names[0]));
+        throw Exception(
+            ErrorCodes::UNKNOWN_TABLE,
+            "Table {}.{} does not exist. Maybe you meant {}.{}?",
+            backQuoteIfNeed(getDatabaseName()),
+            backQuoteIfNeed(name),
+            backQuoteIfNeed(hint.first),
+            backQuoteIfNeed(hint.second));
 }
 
 IDatabase::IDatabase(String database_name_) : database_name(std::move(database_name_))

src/Interpreters/DatabaseCatalog.h

@@ -30,29 +30,6 @@ namespace fs = std::filesystem;
 namespace DB
 {
 
-class TableNameHints : public IHints<>
-{
-public:
-    TableNameHints(ConstDatabasePtr database_, ContextPtr context_)
-        : context(context_),
-        database(database_)
-    {
-    }
-    Names getAllRegisteredNames() const override
-    {
-        Names result;
-        if (database)
-        {
-            for (auto table_it = database->getTablesIterator(context); table_it->isValid(); table_it->next())
-                result.emplace_back(table_it->name());
-        }
-        return result;
-    }
-private:
-    ContextPtr context;
-    ConstDatabasePtr database;
-};
-
 class IDatabase;
 class Exception;
 class ColumnsDescription;
@@ -392,6 +369,68 @@ class DatabaseCatalog : boost::noncopyable, WithMutableContext
     static constexpr time_t DBMS_DEFAULT_DISK_RELOAD_PERIOD_SEC = 5;
 };
 
+class TableNameHints : public IHints<>
+{
+public:
+    TableNameHints(ConstDatabasePtr database_, ContextPtr context_)
+        : context(context_),
+        database(database_)
+    {
+    }
+
+    /// getHintForTable tries to get a hint for the provided table_name in the provided
+    /// database. If the results are empty, it goes for extended hints for the table
+    /// with getExtendedHintForTable which looks for the table name in every database that's
+    /// available in the database catalog. It finally returns a single hint which is the database
+    /// name and table_name pair which is similar to the table_name provided. Perhaps something to
+    /// consider is should we return more than one pair of hint?
+    std::pair<String, String> getHintForTable(const String & table_name) const
+    {
+        auto results = this->getHints(table_name, getAllRegisteredNames());
+        if (results.empty())
+            return getExtendedHintForTable(table_name);
+        return std::make_pair(database->getDatabaseName(), results[0]);
+    }
+
+    /// getExtendedHintsForTable tries to get hint for the given table_name across all
+    /// the databases that are available in the database catalog.
+    std::pair<String, String> getExtendedHintForTable(const String & table_name) const
+    {
+        /// load all available databases from the DatabaseCatalog instance
+        auto & database_catalog = DatabaseCatalog::instance();
+        auto all_databases = database_catalog.getDatabases();
+
+        for (const auto & [db_name, db] : all_databases)
+        {
+            /// this case should be covered already by getHintForTable
+            if (db_name == database->getDatabaseName())
+                continue;
+
+            TableNameHints hints(db, context);
+            auto results = hints.getHints(table_name);
+
+            /// if the results are not empty, return the first instance of the table_name
+            /// and the corresponding database_name that was found.
+            if (!results.empty())
+                return std::make_pair(db_name, results[0]);
+        }
+        return {};
+    }
+
+    Names getAllRegisteredNames() const override
+    {
+        Names result;
+        if (database)
+            for (auto table_it = database->getTablesIterator(context); table_it->isValid(); table_it->next())
+                result.emplace_back(table_it->name());
+        return result;
+    }
+
+private:
+    ContextPtr context;
+    ConstDatabasePtr database;
+};
+
 
 /// This class is useful when creating a table or database.
 /// Usually we create IStorage/IDatabase object first and then add it to IDatabase/DatabaseCatalog.

tests/integration/test_wrong_db_or_table_name/test.py

@@ -61,26 +61,39 @@ def test_wrong_table_name(start):
     node.query(
         """
         CREATE DATABASE test;
+        CREATE DATABASE test2;
         CREATE TABLE test.table_test (i Int64) ENGINE=Memory;
         CREATE TABLE test.table_test2 (i Int64) ENGINE=Memory;
         INSERT INTO test.table_test SELECT 1;
         """
     )
     with pytest.raises(
         QueryRuntimeException,
-        match="DB::Exception: Table test.table_test1 does not exist. Maybe you meant table_test?.",
+        match="DB::Exception: Table test.table_test1 does not exist. Maybe you meant test.table_test?.",
     ):
         node.query(
             """
             SELECT * FROM test.table_test1 LIMIT 1;
             """
         )
     assert int(node.query("SELECT count() FROM test.table_test;")) == 1
+
+    with pytest.raises(
+        QueryRuntimeException,
+        match="DB::Exception: Table test2.table_test1 does not exist. Maybe you meant test.table_test?.",
+    ):
+        node.query(
+            """
+            SELECT * FROM test2.table_test1 LIMIT 1;
+            """
+        )
+
     node.query(
         """
             DROP TABLE test.table_test;
             DROP TABLE test.table_test2;
             DROP DATABASE test;
+            DROP DATABASE test2;
             """
     )
 
@@ -89,20 +102,29 @@ def test_drop_wrong_table_name(start):
     node.query(
         """
         CREATE DATABASE test;
+        CREATE DATABASE test2;
         CREATE TABLE test.table_test (i Int64) ENGINE=Memory;
         INSERT INTO test.table_test SELECT 1;
         """
     )
 
     with pytest.raises(
         QueryRuntimeException,
-        match="DB::Exception: Table test.table_tes does not exist. Maybe you meant table_test?.",
+        match="DB::Exception: Table test.table_test1 does not exist. Maybe you meant test.table_test?.",
     ):
-        node.query("DROP TABLE test.table_tes;")
+        node.query("DROP TABLE test.table_test1;")
     assert int(node.query("SELECT count() FROM test.table_test;")) == 1
+
+    with pytest.raises(
+        QueryRuntimeException,
+        match="DB::Exception: Table test2.table_test does not exist. Maybe you meant test.table_test?.",
+    ):
+        node.query("DROP TABLE test2.table_test;")
+
     node.query(
         """
         DROP TABLE test.table_test;
         DROP DATABASE test;
+        DROP DATABASE test2;
         """
     )