refactor(kqp_table_resolver): add table path to scheme error message (#…

…12027)
ydb-platform · Nov 27, 2024 · 3801e16 · 3801e16
1 parent 423f428
commit 3801e16
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 3 deletions.
diff --git a/ydb/core/kqp/executer_actor/kqp_table_resolver.cpp b/ydb/core/kqp/executer_actor/kqp_table_resolver.cpp
@@ -80,7 +80,7 @@ class TKqpTableResolver : public TActorBootstrapped<TKqpTableResolver> {
             if (entry.Status != NSchemeCache::TSchemeCacheNavigate::EStatus::Ok) {
                 ReplyErrorAndDie(Ydb::StatusIds::SCHEME_ERROR,
                     YqlIssue({}, NYql::TIssuesIds::KIKIMR_SCHEME_MISMATCH, TStringBuilder()
-                        << "Failed to resolve table " << entry.TableId << " keys: " << entry.Status << '.'));
+                        << "Failed to resolve table with tableId: " << entry.TableId << " status: " << entry.Status << '.'));
                 return;
             }
 
@@ -114,12 +114,16 @@ class TKqpTableResolver : public TActorBootstrapped<TKqpTableResolver> {
                 LOG_E("Error resolving keys for entry: " << entry.ToString(*AppData()->TypeRegistry));
 
                 TStringBuilder path;
-                path << "unresolved `" << entry.KeyDescription->TableId << '`';
+                if (auto it = TablePathsById.find(entry.KeyDescription->TableId); it != TablePathsById.end()) {
+                    path << '`' << it->second << '`';
+                } else {
+                    path << "with unknown path, tableId: `" << entry.KeyDescription->TableId << '`';
+                }
 
                 timer.reset();
                 ReplyErrorAndDie(Ydb::StatusIds::SCHEME_ERROR,
                     YqlIssue({}, NYql::TIssuesIds::KIKIMR_SCHEME_MISMATCH, TStringBuilder()
-                        << "Failed to resolve table " << path << " keys: " << entry.Status << '.'));
+                        << "Failed to resolve table " << path << " status: " << entry.Status << '.'));
                 return;
             }
 
@@ -164,6 +168,7 @@ class TKqpTableResolver : public TActorBootstrapped<TKqpTableResolver> {
                 for (const auto& operation : stageInfo.Meta.ShardOperations) {
                     const auto& tableInfo = stageInfo.Meta.TableConstInfo;
                     Y_ENSURE(tableInfo);
+                    TablePathsById.emplace(stageInfo.Meta.TableId, tableInfo->Path);
                     stageInfo.Meta.TableKind = tableInfo->TableKind;
 
                     stageInfo.Meta.ShardKey = ExtractKey(stageInfo.Meta.TableId, stageInfo.Meta.TableConstInfo, operation);
@@ -258,6 +263,7 @@ class TKqpTableResolver : public TActorBootstrapped<TKqpTableResolver> {
     TIntrusiveConstPtr<NACLib::TUserToken> UserToken;
     const TVector<IKqpGateway::TPhysicalTxData>& Transactions;
     THashMap<TTableId, TVector<TStageId>> TableRequestIds;
+    THashMap<TTableId, TString> TablePathsById;
     bool NavigationFinished = false;
     bool ResolvingFinished = false;
 

diff --git a/ydb/core/kqp/ut/scheme/kqp_acl_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_acl_ut.cpp
@@ -90,6 +90,34 @@ Y_UNIT_TEST_SUITE(KqpAcl) {
         driver.Stop(true);
     }
 
+    Y_UNIT_TEST(FailedReadAccessDenied) {
+        TKikimrRunner kikimr;
+        {
+            NYdb::NScheme::TPermissions permissions("user0@builtin",{});
+            auto schemeClient = kikimr.GetSchemeClient();
+            auto result = schemeClient.ModifyPermissions("/Root/TwoShard",
+                NYdb::NScheme::TModifyPermissionsSettings().AddGrantPermissions(permissions)
+            ).ExtractValueSync();
+            AssertSuccessResult(result);
+        }
+
+        auto driverConfig = TDriverConfig()
+            .SetEndpoint(kikimr.GetEndpoint())
+            .SetAuthToken("user0@builtin");
+        auto driver = TDriver(driverConfig);
+        auto db = NYdb::NTable::TTableClient(driver);
+        auto session = db.CreateSession().GetValueSync().GetSession();
+
+        auto result = session.ExecuteDataQuery(R"(
+            SELECT * FROM `/Root/TwoShard`;
+        )", TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx()).ExtractValueSync();
+        Cerr << result.GetIssues().ToString() << Endl;
+        UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::SCHEME_ERROR);
+        const auto expectedIssueMessage = "Cannot find table 'db.[/Root/TwoShard]' because it does not exist or you do not have access permissions.";
+        UNIT_ASSERT_VALUES_EQUAL(result.GetIssues().ToString().Contains(expectedIssueMessage), true);
+        driver.Stop(true);
+    }
+
     Y_UNIT_TEST(WriteSuccess) {
         TKikimrRunner kikimr;
         {
@@ -119,6 +147,36 @@ Y_UNIT_TEST_SUITE(KqpAcl) {
         driver.Stop(true);
     }
 
+    Y_UNIT_TEST(FailedWriteAccessDenied) {
+        TKikimrRunner kikimr;
+        {
+            NYdb::NScheme::TPermissions permissions("user0@builtin",
+                {"ydb.deprecated.describe_schema", "ydb.deprecated.select_row"}
+            );
+            auto schemeClient = kikimr.GetSchemeClient();
+            auto result = schemeClient.ModifyPermissions("/Root/TwoShard",
+                NYdb::NScheme::TModifyPermissionsSettings().AddGrantPermissions(permissions)
+            ).ExtractValueSync();
+            AssertSuccessResult(result);
+        }
+
+        auto driverConfig = TDriverConfig()
+            .SetEndpoint(kikimr.GetEndpoint())
+            .SetAuthToken("user0@builtin");
+        auto driver = TDriver(driverConfig);
+        auto db = NYdb::NTable::TTableClient(driver);
+        auto session = db.CreateSession().GetValueSync().GetSession();
+
+        auto result = session.ExecuteDataQuery(R"(
+            UPSERT INTO `/Root/TwoShard` (Key, Value1, Value2) VALUES
+                (10u, "One", -10);
+        )", TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx()).ExtractValueSync();
+        UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::ABORTED);
+        const auto expectedIssueMessage = "Failed to resolve table `/Root/TwoShard` status: AccessDenied.";
+        UNIT_ASSERT_VALUES_EQUAL(result.GetIssues().ToString().Contains(expectedIssueMessage), true);
+        driver.Stop(true);
+    }
+
     Y_UNIT_TEST(RecursiveCreateTableShouldSuccess) {
         TKikimrRunner kikimr;
         {