Skip to content

Commit 6706ab0

Browse files
GrigoriyPAGrigoriyPA
committed
YQ-4170 fixed external table alter (#16315)
1 parent 05c3e3f commit 6706ab0

File tree

3 files changed

+90
-36
lines changed

3 files changed

+90
-36
lines changed

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ class TPropose: public TSubOperationState {
6161
Y_ABORT_UNLESS(txState);
6262
Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable);
6363

64-
const auto pathId = txState->TargetPathId;
65-
const auto dataSourcePathId = txState->SourcePathId;
66-
const auto path = TPath::Init(pathId, context.SS);
64+
const auto pathId = txState->TargetPathId;
65+
const auto dataSourcePathId = txState->SourcePathId;
66+
const auto path = TPath::Init(pathId, context.SS);
6767
const TPathElement::TPtr pathPtr = context.SS->PathsById.at(pathId);
6868
const TPathElement::TPtr dataSourcePathPtr =
6969
context.SS->PathsById.at(dataSourcePathId);
@@ -268,6 +268,7 @@ class TAlterExternalTable: public TSubOperation {
268268
NIceDb::TNiceDb& db,
269269
const TPathElement::TPtr& externalTable,
270270
const TExternalTableInfo::TPtr& externalTableInfo,
271+
const TExternalTableInfo::TPtr& oldExternalTableInfo,
271272
const TPathId& externalDataSourcePathId,
272273
const TExternalDataSourceInfo::TPtr& externalDataSource,
273274
const TPathId& oldExternalDataSourcePathId,
@@ -276,7 +277,6 @@ class TAlterExternalTable: public TSubOperation {
276277
bool isSameDataSource) const {
277278
context.SS->ExternalTables[externalTable->PathId] = externalTableInfo;
278279

279-
280280
if (!acl.empty()) {
281281
externalTable->ApplyACL(acl);
282282
}
@@ -286,6 +286,13 @@ class TAlterExternalTable: public TSubOperation {
286286
context.SS->PersistExternalDataSource(db, externalDataSourcePathId, externalDataSource);
287287
context.SS->PersistExternalDataSource(db, oldExternalDataSourcePathId, oldExternalDataSource);
288288
}
289+
290+
for (const auto& [oldColId, _] : oldExternalTableInfo->Columns) {
291+
if (!externalTableInfo->Columns.contains(oldColId)) {
292+
db.Table<Schema::MigratedColumns>().Key(externalTable->PathId.OwnerId, externalTable->PathId.LocalPathId, oldColId).Delete();
293+
}
294+
}
295+
289296
context.SS->PersistExternalTable(db, externalTable->PathId, externalTableInfo);
290297
context.SS->PersistTxState(db, OperationId);
291298
}
@@ -303,7 +310,7 @@ class TAlterExternalTable: public TSubOperation {
303310

304311
LOG_N("TAlterExternalTable Propose"
305312
<< ": opId# " << OperationId
306-
<< ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists:" << externalTableDescription.GetReplaceIfExists());
313+
<< ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists: " << externalTableDescription.GetReplaceIfExists());
307314

308315
auto result = MakeHolder<TProposeResponse>(NKikimrScheme::StatusAccepted,
309316
static_cast<ui64>(OperationId.GetTxId()),
@@ -320,7 +327,7 @@ class TAlterExternalTable: public TSubOperation {
320327
RETURN_RESULT_UNLESS(NExternalTable::IsParentPathValid(result, parentPath));
321328

322329
const TString acl = Transaction.GetModifyACL().GetDiffACL();
323-
TPath dstPath = parentPath.Child(name);
330+
TPath dstPath = parentPath.Child(name);
324331
RETURN_RESULT_UNLESS(IsDestinationPathValid(result, dstPath, acl));
325332

326333
const auto dataSourcePath =
@@ -384,7 +391,7 @@ class TAlterExternalTable: public TSubOperation {
384391
oldDataSource,
385392
IsSameDataSource);
386393

387-
PersistExternalTable(context, db, externalTable, externalTableInfo,
394+
PersistExternalTable(context, db, externalTable, externalTableInfo, oldExternalTableInfo,
388395
dataSourcePath->PathId, externalDataSource,
389396
OldDataSourcePathId, oldDataSource, acl,
390397
IsSameDataSource);

‎ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,21 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) {
313313
)", {{NKikimrScheme::StatusPathDoesNotExist, "Check failed: path: '/MyRoot/ExternalDataSource1'"}});
314314
}
315315

316+
std::vector<NKikimrSchemeOp::TColumnDescription> CheckExternalTable(TTestBasicRuntime& runtime, ui64 version, const TString& location) {
317+
auto describeResult = DescribePath(runtime, "/MyRoot/ExternalTable");
318+
TestDescribeResult(describeResult, {NLs::PathExist});
319+
UNIT_ASSERT(describeResult.GetPathDescription().HasExternalTableDescription());
320+
const auto& externalTableDescription = describeResult.GetPathDescription().GetExternalTableDescription();
321+
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetName(), "ExternalTable");
322+
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetDataSourcePath(), "/MyRoot/ExternalDataSource");
323+
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetLocation(), location);
324+
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetSourceType(), "ObjectStorage");
325+
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetVersion(), version);
326+
327+
auto& columns = externalTableDescription.GetColumns();
328+
return {columns.begin(), columns.end()};
329+
}
330+
316331
Y_UNIT_TEST(ReplaceExternalTableIfNotExists) {
317332
TTestBasicRuntime runtime;
318333
TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true).RunFakeConfigDispatcher(true));
@@ -331,20 +346,11 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) {
331346
env.TestWaitNotification(runtime, txId);
332347

333348
{
334-
auto describeResult = DescribePath(runtime, "/MyRoot/ExternalTable");
335-
TestDescribeResult(describeResult, {NLs::PathExist});
336-
UNIT_ASSERT(describeResult.GetPathDescription().HasExternalTableDescription());
337-
const auto& externalTableDescription = describeResult.GetPathDescription().GetExternalTableDescription();
338-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetName(), "ExternalTable");
339-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetDataSourcePath(), "/MyRoot/ExternalDataSource");
340-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetLocation(), "/");
341-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetSourceType(), "ObjectStorage");
342-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetVersion(), 1);
343-
auto& columns = externalTableDescription.GetColumns();
349+
const auto& columns = CheckExternalTable(runtime, 1, "/");
344350
UNIT_ASSERT_VALUES_EQUAL(columns.size(), 1);
345-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetName(), "key");
346-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetType(), "Uint64");
347-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetNotNull(), false);
351+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetName(), "key");
352+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetType(), "Uint64");
353+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetNotNull(), false);
348354
}
349355

350356
TestCreateExternalTable(runtime, ++txId, "/MyRoot", R"(
@@ -359,23 +365,32 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) {
359365
env.TestWaitNotification(runtime, txId);
360366

361367
{
362-
auto describeResult = DescribePath(runtime, "/MyRoot/ExternalTable");
363-
TestDescribeResult(describeResult, {NLs::PathExist});
364-
UNIT_ASSERT(describeResult.GetPathDescription().HasExternalTableDescription());
365-
const auto& externalTableDescription = describeResult.GetPathDescription().GetExternalTableDescription();
366-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetName(), "ExternalTable");
367-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetDataSourcePath(), "/MyRoot/ExternalDataSource");
368-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetLocation(), "/new_location");
369-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetSourceType(), "ObjectStorage");
370-
UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetVersion(), 2);
371-
auto& columns = externalTableDescription.GetColumns();
368+
const auto& columns = CheckExternalTable(runtime, 2, "/new_location");
372369
UNIT_ASSERT_VALUES_EQUAL(columns.size(), 2);
373-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetName(), "key");
374-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetType(), "Uint64");
375-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetNotNull(), false);
376-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetName(), "value");
377-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetType(), "Uint64");
378-
UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetNotNull(), false);
370+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetName(), "key");
371+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetType(), "Uint64");
372+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetNotNull(), false);
373+
UNIT_ASSERT_VALUES_EQUAL(columns[1].GetName(), "value");
374+
UNIT_ASSERT_VALUES_EQUAL(columns[1].GetType(), "Uint64");
375+
UNIT_ASSERT_VALUES_EQUAL(columns[1].GetNotNull(), false);
376+
}
377+
378+
TestCreateExternalTable(runtime, ++txId, "/MyRoot", R"(
379+
Name: "ExternalTable"
380+
SourceType: "General"
381+
DataSourcePath: "/MyRoot/ExternalDataSource"
382+
Location: "/other_location"
383+
Columns { Name: "value" Type: "Uint64" }
384+
ReplaceIfExists: true
385+
)", {NKikimrScheme::StatusAccepted});
386+
env.TestWaitNotification(runtime, txId);
387+
388+
{
389+
const auto& columns = CheckExternalTable(runtime, 3, "/other_location");
390+
UNIT_ASSERT_VALUES_EQUAL(columns.size(), 1);
391+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetName(), "value");
392+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetType(), "Uint64");
393+
UNIT_ASSERT_VALUES_EQUAL(columns[0].GetNotNull(), false);
379394
}
380395
}
381396

‎ydb/core/tx/schemeshard/ut_external_table_reboots/ut_external_table_reboots.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,36 @@ Y_UNIT_TEST_SUITE(TExternalTableTestReboots) {
277277
}
278278
});
279279
}
280+
281+
Y_UNIT_TEST(DropReplacedExternalTableWithReboots) {
282+
TTestWithReboots t;
283+
t.GetTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true).RunFakeConfigDispatcher(true);
284+
285+
t.Run([&](TTestActorRuntime& runtime, bool&) {
286+
CreateExternalDataSource(runtime, *t.TestEnv, ++t.TxId);
287+
TestCreateExternalTable(runtime, ++t.TxId, "/MyRoot", R"(
288+
Name: "ExternalTable"
289+
SourceType: "General"
290+
DataSourcePath: "/MyRoot/ExternalDataSource"
291+
Location: "/"
292+
Columns { Name: "RowId" Type: "Uint64"}
293+
Columns { Name: "FirstValue" Type: "Utf8"}
294+
Columns { Name: "SecondValue" Type: "Utf8"}
295+
)");
296+
t.TestEnv->TestWaitNotification(runtime, t.TxId);
297+
298+
TestCreateExternalTable(runtime, ++t.TxId, "/MyRoot", R"(
299+
Name: "ExternalTable"
300+
SourceType: "General"
301+
DataSourcePath: "/MyRoot/ExternalDataSource"
302+
Location: "/"
303+
Columns { Name: "RowId" Type: "Uint64"}
304+
ReplaceIfExists: true
305+
)");
306+
t.TestEnv->TestWaitNotification(runtime, t.TxId);
307+
308+
TestDropExternalTable(runtime, ++t.TxId, "/MyRoot", "ExternalTable");
309+
t.TestEnv->TestWaitNotification(runtime, t.TxId);
310+
});
311+
}
280312
}

0 commit comments

Comments
 (0)