Skip to content

Commit

Permalink
Merge 12e783e into 9bce709
Browse files Browse the repository at this point in the history
  • Loading branch information
@shnikd
shnikd authored May 16, 2024
2 parents 9bce709 + 12e783e commit 95b353d
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 13 deletions.
51 changes: 45 additions & 6 deletions ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ bool IsSuperUser(const NACLib::TUserToken* userToken) {

TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table, const NKikimrSchemeOp::TTableDescription& alter,
const bool shadowDataAllowed,
TString& errStr, NKikimrScheme::EStatus& status, TOperationContext& context) {
TString& errStr, NKikimrScheme::EStatus& status, TOperationContext& context,
const THashSet<TString>& localSequences) {
const TAppData* appData = AppData(context.Ctx);

if (!path.IsCommonSensePath()) {
Expand Down Expand Up @@ -132,7 +133,9 @@ TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table,

const TSubDomainInfo& subDomain = *path.DomainInfo();
const TSchemeLimits& limits = subDomain.GetSchemeLimits();
TTableInfo::TAlterDataPtr alterData = TTableInfo::CreateAlterData(table, copyAlter, *appData->TypeRegistry, limits, subDomain, context.SS->EnableTablePgTypes, errStr);


TTableInfo::TAlterDataPtr alterData = TTableInfo::CreateAlterData(table, copyAlter, *appData->TypeRegistry, limits, subDomain, context.SS->EnableTablePgTypes, errStr, localSequences);
if (!alterData) {
status = NKikimrScheme::StatusInvalidParameter;
return nullptr;
Expand Down Expand Up @@ -410,6 +413,7 @@ class TPropose: public TSubOperationState {

class TAlterTable: public TSubOperation {
bool AllowShadowData = false;
THashSet<TString> LocalSequences;

static TTxState::ETxState NextState() {
return TTxState::CreateParts;
Expand Down Expand Up @@ -460,6 +464,10 @@ class TAlterTable: public TSubOperation {
return AllowShadowData || AppData()->AllowShadowDataInSchemeShardForTests;
}

void SetLocalSequences(const THashSet<TString>& localSequences) {
LocalSequences = localSequences;
}

THolder<TProposeResponse> Propose(const TString&, TOperationContext& context) override {
const TTabletId ssId = context.SS->SelfTabletId();

Expand Down Expand Up @@ -560,7 +568,7 @@ class TAlterTable: public TSubOperation {
}

NKikimrScheme::EStatus status;
TTableInfo::TAlterDataPtr alterData = ParseParams(path, table, alter, IsShadowDataAllowed(), errStr, status, context);
TTableInfo::TAlterDataPtr alterData = ParseParams(path, table, alter, IsShadowDataAllowed(), errStr, status, context, LocalSequences);
if (!alterData) {
result->SetError(status, errStr);
return result;
Expand Down Expand Up @@ -621,8 +629,11 @@ class TAlterTable: public TSubOperation {

namespace NKikimr::NSchemeShard {

ISubOperation::TPtr CreateAlterTable(TOperationId id, const TTxTransaction& tx) {
return MakeSubOperation<TAlterTable>(id, tx);
ISubOperation::TPtr CreateAlterTable(
TOperationId id, const TTxTransaction& tx, const THashSet<TString>& localSequences) {
auto obj = MakeSubOperation<TAlterTable>(id, tx);
static_cast<TAlterTable*>(obj.Get())->SetLocalSequences(localSequences);
return obj;
}

ISubOperation::TPtr CreateAlterTable(TOperationId id, TTxState::ETxState state) {
Expand Down Expand Up @@ -673,7 +684,35 @@ TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const T
}

if (path.IsCommonSensePath()) {
return {CreateAlterTable(id, tx)};
const auto& alter = tx.GetAlterTable();

std::optional<TString> defaultFromSequence;
for (const auto& column: alter.GetColumns()) {
if (column.HasDefaultFromSequence()) {
defaultFromSequence = column.GetDefaultFromSequence();
}
}
Y_ABORT_UNLESS(!defaultFromSequence.has_value() || (alter.GetColumns().size() == 1));

const auto sequencePath = TPath::Resolve(*defaultFromSequence, context.SS);
{
const auto checks = sequencePath.Check();
checks
.NotEmpty()
.NotUnderDomainUpgrade()
.IsAtLocalSchemeShard()
.IsResolved()
.NotDeleted()
.IsSequence()
.NotUnderDeleting()
.NotUnderOperation();

if (!checks) {
return {CreateReject(id, checks.GetStatus(), checks.GetError())};
}
}

return {CreateAlterTable(id, tx, { sequencePath.PathString() })};
}

TPath parent = path.Parent();
Expand Down
3 changes: 2 additions & 1 deletion ydb/core/tx/schemeshard/schemeshard__operation_part.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ ISubOperation::TPtr CreateCopyTable(TOperationId id, const TTxTransaction& tx,
ISubOperation::TPtr CreateCopyTable(TOperationId id, TTxState::ETxState state);
TVector<ISubOperation::TPtr> CreateCopyTable(TOperationId nextId, const TTxTransaction& tx, TOperationContext& context);

ISubOperation::TPtr CreateAlterTable(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateAlterTable(
TOperationId id, const TTxTransaction& tx, const THashSet<TString>& localSequences = {});
ISubOperation::TPtr CreateAlterTable(TOperationId id, TTxState::ETxState state);
TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context);

Expand Down
41 changes: 35 additions & 6 deletions ydb/core/tx/schemeshard/schemeshard_info_types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,20 +269,47 @@ TTableInfo::TAlterDataPtr TTableInfo::CreateAlterData(
return nullptr;
}

if (!columnFamily) {
if (!columnFamily && !col.HasDefaultFromSequence()) {
errStr = Sprintf("Nothing to alter for column '%s'", colName.data());
return nullptr;
}

if (col.DefaultValue_case() != NKikimrSchemeOp::TColumnDescription::DEFAULTVALUE_NOT_SET) {
errStr = Sprintf("Cannot alter default for column '%s'", colName.c_str());
return nullptr;
if (col.HasDefaultFromSequence()) {
if (!localSequences.contains(col.GetDefaultFromSequence())) {
errStr = Sprintf("Column '%s' cannot use an unknown sequence '%s'", colName.c_str(), col.GetDefaultFromSequence().c_str());
return nullptr;
}
} else {
if (col.DefaultValue_case() != NKikimrSchemeOp::TColumnDescription::DEFAULTVALUE_NOT_SET) {
errStr = Sprintf("Cannot set default from literal for column '%s'", colName.c_str());
return nullptr;
}
}

ui32 colId = colName2Id[colName];
const TTableInfo::TColumn& sourceColumn = source->Columns[colId];

if (col.HasDefaultFromSequence()) {
if (sourceColumn.PType.GetTypeId() != NScheme::NTypeIds::Int64) {
errStr = Sprintf(
"Sequence value type '%s' must be equal to the column type '%s'", "Int64",
NScheme::TypeName(sourceColumn.PType, sourceColumn.PTypeMod).c_str());
return nullptr;
}
}

TTableInfo::TColumn& column = alterData->Columns[colId];
column = source->Columns[colId];
column.Family = columnFamily->GetId();
column = sourceColumn;
if (columnFamily) {
column.Family = columnFamily->GetId();
}
if (col.HasDefaultFromSequence()) {
column.DefaultKind = ETableColumnDefaultKind::FromSequence;
column.DefaultValue = col.GetDefaultFromSequence();
} else if (col.HasDefaultFromLiteral()) {
column.DefaultKind = ETableColumnDefaultKind::FromLiteral;
column.DefaultValue = col.GetDefaultFromLiteral().SerializeAsString();
}
} else {
if (colName2Id.contains(colName)) {
errStr = Sprintf("Column '%s' specified more than once", colName.data());
Expand Down Expand Up @@ -1338,6 +1365,8 @@ void TTableInfo::FinishAlter() {
//oldCol->CreateVersion = col.second.CreateVersion;
oldCol->DeleteVersion = col.second.DeleteVersion;
oldCol->Family = col.second.Family;
oldCol->DefaultKind = col.second.DefaultKind;
oldCol->DefaultValue = col.second.DefaultValue;
} else {
Columns[col.first] = col.second;
if (col.second.KeyOrder != (ui32)-1) {
Expand Down
6 changes: 6 additions & 0 deletions ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,12 @@ TCheckFunc IndexDataColumns(const TVector<TString>& dataColumnNames) {
};
}

TCheckFunc SequenceName(const TString& name) {
return [=] (const NKikimrScheme::TEvDescribeSchemeResult& record) {
UNIT_ASSERT_VALUES_EQUAL(record.GetPathDescription().GetSequenceDescription().GetName(), name);
};
}

TCheckFunc SequenceIncrement(i64 increment) {
return [=] (const NKikimrScheme::TEvDescribeSchemeResult& record) {
UNIT_ASSERT_VALUES_EQUAL(record.GetPathDescription().GetSequenceDescription().GetIncrement(), increment);
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_helpers/ls_checks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ namespace NLs {
TCheckFunc IndexKeys(const TVector<TString>& keyNames);
TCheckFunc IndexDataColumns(const TVector<TString>& dataColumnNames);

TCheckFunc SequenceName(const TString& name);
TCheckFunc SequenceIncrement(i64 increment);
TCheckFunc SequenceMaxValue(i64 maxValue);
TCheckFunc SequenceMinValue(i64 minValue);
Expand Down
145 changes: 145 additions & 0 deletions ydb/core/tx/schemeshard/ut_sequence/ut_sequence.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,149 @@ Y_UNIT_TEST_SUITE(TSequence) {
)", {NKikimrScheme::StatusInvalidParameter});
}

Y_UNIT_TEST(AlterTableSetDefaultFromSequence) {
TTestBasicRuntime runtime;
TTestEnv env(runtime);
ui64 txId = 100;

runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NActors::NLog::PRI_TRACE);
runtime.SetLogPriority(NKikimrServices::SEQUENCESHARD, NActors::NLog::PRI_TRACE);

TestCreateIndexedTable(runtime, ++txId, "/MyRoot", R"(
TableDescription {
Name: "Table1"
Columns { Name: "key" Type: "Int64" }
Columns { Name: "value1" Type: "Int64" }
Columns { Name: "value2" Type: "Int32" }
KeyColumnNames: ["key"]
}
)");

TestCreateTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table2"
Columns { Name: "key" Type: "Int64" }
Columns { Name: "value1" Type: "Int64" }
Columns { Name: "value2" Type: "Int64" }
KeyColumnNames: ["key"]
)");

env.TestWaitNotification(runtime, txId);

TestCreateSequence(runtime, ++txId, "/MyRoot", R"(
Name: "seq1"
)");
env.TestWaitNotification(runtime, txId);

TestCreateSequence(runtime, ++txId, "/MyRoot", R"(
Name: "seq2"
)");
env.TestWaitNotification(runtime, txId);

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table1"
Columns { Name: "key" DefaultFromSequence: "/MyRoot/seq1" }
)", {TEvSchemeShard::EStatus::StatusInvalidParameter});

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table1"
Columns { Name: "value1" DefaultFromSequence: "/MyRoot/seq1" }
)");
env.TestWaitNotification(runtime, txId);

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table1"
Columns { Name: "value2" DefaultFromSequence: "/MyRoot/seq1" }
)", {TEvSchemeShard::EStatus::StatusInvalidParameter});

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table2"
Columns { Name: "value1" DefaultFromSequence: "/MyRoot/seq1" }
)");
env.TestWaitNotification(runtime, txId);

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table2"
Columns { Name: "value2" DefaultFromSequence: "/MyRoot/seq1" }
)");
env.TestWaitNotification(runtime, txId);

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table2"
Columns { Name: "value1" DefaultFromSequence: "/MyRoot/seq3" }
)", {TEvSchemeShard::EStatus::StatusPathDoesNotExist});

auto table1 = DescribePath(runtime, "/MyRoot/Table1")
.GetPathDescription()
.GetTable();

for (const auto& column: table1.GetColumns()) {
if (column.GetName() == "value1") {
UNIT_ASSERT(column.HasDefaultFromSequence());
UNIT_ASSERT_VALUES_EQUAL(column.GetDefaultFromSequence(), "/MyRoot/seq1");

TestDescribeResult(DescribePath(runtime, column.GetDefaultFromSequence()),
{
NLs::SequenceName("seq1"),
}
);
break;
}
}

auto table2 = DescribePath(runtime, "/MyRoot/Table2")
.GetPathDescription()
.GetTable();

for (const auto& column: table2.GetColumns()) {
if (column.GetName() == "key") {
continue;
}
UNIT_ASSERT(column.HasDefaultFromSequence());
UNIT_ASSERT_VALUES_EQUAL(column.GetDefaultFromSequence(), "/MyRoot/seq1");

TestDescribeResult(DescribePath(runtime, column.GetDefaultFromSequence()),
{
NLs::SequenceName("seq1"),
}
);
}

TestAlterTable(runtime, ++txId, "/MyRoot", R"(
Name: "Table2"
Columns { Name: "value1" DefaultFromSequence: "/MyRoot/seq2" }
)");
env.TestWaitNotification(runtime, txId);

table2 = DescribePath(runtime, "/MyRoot/Table2")
.GetPathDescription()
.GetTable();

for (const auto& column: table2.GetColumns()) {
if (column.GetName() == "key") {
continue;
}
if (column.GetName() == "value1") {
UNIT_ASSERT(column.HasDefaultFromSequence());
UNIT_ASSERT_VALUES_EQUAL(column.GetDefaultFromSequence(), "/MyRoot/seq2");

TestDescribeResult(DescribePath(runtime, column.GetDefaultFromSequence()),
{
NLs::SequenceName("seq2"),
}
);
break;
} else if (column.GetName() == "value2") {
UNIT_ASSERT(column.HasDefaultFromSequence());
UNIT_ASSERT_VALUES_EQUAL(column.GetDefaultFromSequence(), "/MyRoot/seq1");

TestDescribeResult(DescribePath(runtime, column.GetDefaultFromSequence()),
{
NLs::SequenceName("seq1"),
}
);
break;
}
}
}

} // Y_UNIT_TEST_SUITE(TSequence)

0 comments on commit 95b353d

Please sign in to comment.