From 63acbfd6bb365c03ba6b5d8b38dc6a7c7ef0988a Mon Sep 17 00:00:00 2001 From: azevaykin <145343289+azevaykin@users.noreply.github.com> Date: Thu, 17 Apr 2025 09:15:04 +0300 Subject: [PATCH 1/2] Feature flags for columns types should be read from AppData() (#17289) --- .../schemeshard/schemeshard__operation_alter_sequence.cpp | 2 +- .../tx/schemeshard/schemeshard__operation_alter_table.cpp | 6 +++--- .../tx/schemeshard/schemeshard__operation_copy_table.cpp | 6 +++--- .../schemeshard/schemeshard__operation_create_sequence.cpp | 2 +- .../tx/schemeshard/schemeshard__operation_create_table.cpp | 6 +++--- ydb/core/tx/schemeshard/schemeshard_impl.cpp | 6 ------ ydb/core/tx/schemeshard/schemeshard_impl.h | 3 --- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_sequence.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_sequence.cpp index d2da3b45f850..cd43d916b24d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_sequence.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_sequence.cpp @@ -488,7 +488,7 @@ class TAlterSequence: public TSubOperation { const NScheme::TTypeRegistry* typeRegistry = AppData()->TypeRegistry; auto description = GetAlterSequenceDescription( - sequenceInfo->Description, sequenceAlter, *typeRegistry, context.SS->EnableTablePgTypes, errStr); + sequenceInfo->Description, sequenceAlter, *typeRegistry, AppData()->FeatureFlags.GetEnableTablePgTypes(), errStr); if (!description) { status = NKikimrScheme::StatusInvalidParameter; result->SetError(status, errStr); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp index 155b5e1339ed..d8fa68a5ac78 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp @@ -145,9 +145,9 @@ TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table, const TSubDomainInfo& subDomain = *path.DomainInfo(); const TSchemeLimits& limits = subDomain.GetSchemeLimits(); const TTableInfo::TCreateAlterDataFeatureFlags featureFlags = { - .EnableTablePgTypes = context.SS->EnableTablePgTypes, - .EnableTableDatetime64 = context.SS->EnableTableDatetime64, - .EnableParameterizedDecimal = context.SS->EnableParameterizedDecimal, + .EnableTablePgTypes = AppData()->FeatureFlags.GetEnableTablePgTypes(), + .EnableTableDatetime64 = AppData()->FeatureFlags.GetEnableTableDatetime64(), + .EnableParameterizedDecimal = AppData()->FeatureFlags.GetEnableParameterizedDecimal(), }; diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp index 5c3bfb8047ca..13a2e0cd3a5f 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp @@ -579,9 +579,9 @@ class TCopyTable: public TSubOperation { const NScheme::TTypeRegistry* typeRegistry = AppData()->TypeRegistry; const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); const TTableInfo::TCreateAlterDataFeatureFlags featureFlags = { - .EnableTablePgTypes = context.SS->EnableTablePgTypes, - .EnableTableDatetime64 = context.SS->EnableTableDatetime64, - .EnableParameterizedDecimal = context.SS->EnableParameterizedDecimal, + .EnableTablePgTypes = AppData()->FeatureFlags.GetEnableTablePgTypes(), + .EnableTableDatetime64 = AppData()->FeatureFlags.GetEnableTableDatetime64(), + .EnableParameterizedDecimal = AppData()->FeatureFlags.GetEnableParameterizedDecimal(), }; TTableInfo::TAlterDataPtr alterData = TTableInfo::CreateAlterData(nullptr, schema, *typeRegistry, limits, *domainInfo, featureFlags, errStr, LocalSequences); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp index 0e23348c0880..9fdb1e8d66b7 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp @@ -508,7 +508,7 @@ class TCreateSequence : public TSubOperation { TSequenceInfo::TPtr alterData = sequenceInfo->CreateNextVersion(); const NScheme::TTypeRegistry* typeRegistry = AppData()->TypeRegistry; auto description = FillSequenceDescription( - descr, *typeRegistry, context.SS->EnableTablePgTypes, errStr); + descr, *typeRegistry, AppData()->FeatureFlags.GetEnableTablePgTypes(), errStr); if (!description) { status = NKikimrScheme::StatusInvalidParameter; result->SetError(status, errStr); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_table.cpp index d43c27aee881..ec28d65e1887 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_table.cpp @@ -578,9 +578,9 @@ class TCreateTable: public TSubOperation { const NScheme::TTypeRegistry* typeRegistry = AppData()->TypeRegistry; const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); const TTableInfo::TCreateAlterDataFeatureFlags featureFlags = { - .EnableTablePgTypes = context.SS->EnableTablePgTypes, - .EnableTableDatetime64 = context.SS->EnableTableDatetime64, - .EnableParameterizedDecimal = context.SS->EnableParameterizedDecimal, + .EnableTablePgTypes = AppData()->FeatureFlags.GetEnableTablePgTypes(), + .EnableTableDatetime64 = AppData()->FeatureFlags.GetEnableTableDatetime64(), + .EnableParameterizedDecimal = AppData()->FeatureFlags.GetEnableParameterizedDecimal(), }; TTableInfo::TAlterDataPtr alterData = TTableInfo::CreateAlterData( nullptr, diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp index b98923726396..6bed664e4071 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_impl.cpp @@ -4648,14 +4648,11 @@ void TSchemeShard::OnActivateExecutor(const TActorContext &ctx) { EnableAlterDatabaseCreateHiveFirst = appData->FeatureFlags.GetEnableAlterDatabaseCreateHiveFirst(); EnablePQConfigTransactionsAtSchemeShard = appData->FeatureFlags.GetEnablePQConfigTransactionsAtSchemeShard(); EnableStatistics = appData->FeatureFlags.GetEnableStatistics(); - EnableTablePgTypes = appData->FeatureFlags.GetEnableTablePgTypes(); EnableServerlessExclusiveDynamicNodes = appData->FeatureFlags.GetEnableServerlessExclusiveDynamicNodes(); EnableAddColumsWithDefaults = appData->FeatureFlags.GetEnableAddColumsWithDefaults(); EnableReplaceIfExistsForExternalEntities = appData->FeatureFlags.GetEnableReplaceIfExistsForExternalEntities(); EnableTempTables = appData->FeatureFlags.GetEnableTempTables(); - EnableTableDatetime64 = appData->FeatureFlags.GetEnableTableDatetime64(); EnableVectorIndex = appData->FeatureFlags.GetEnableVectorIndex(); - EnableParameterizedDecimal = appData->FeatureFlags.GetEnableParameterizedDecimal(); EnableDataErasure = appData->FeatureFlags.GetEnableDataErasure(); ConfigureCompactionQueues(appData->CompactionConfig, ctx); @@ -7284,16 +7281,13 @@ void TSchemeShard::ApplyConsoleConfigs(const NKikimrConfig::TFeatureFlags& featu EnableAlterDatabaseCreateHiveFirst = featureFlags.GetEnableAlterDatabaseCreateHiveFirst(); EnablePQConfigTransactionsAtSchemeShard = featureFlags.GetEnablePQConfigTransactionsAtSchemeShard(); EnableStatistics = featureFlags.GetEnableStatistics(); - EnableTablePgTypes = featureFlags.GetEnableTablePgTypes(); EnableServerlessExclusiveDynamicNodes = featureFlags.GetEnableServerlessExclusiveDynamicNodes(); EnableAddColumsWithDefaults = featureFlags.GetEnableAddColumsWithDefaults(); EnableTempTables = featureFlags.GetEnableTempTables(); EnableReplaceIfExistsForExternalEntities = featureFlags.GetEnableReplaceIfExistsForExternalEntities(); - EnableTableDatetime64 = featureFlags.GetEnableTableDatetime64(); EnableResourcePoolsOnServerless = featureFlags.GetEnableResourcePoolsOnServerless(); EnableVectorIndex = featureFlags.GetEnableVectorIndex(); EnableExternalDataSourcesOnServerless = featureFlags.GetEnableExternalDataSourcesOnServerless(); - EnableParameterizedDecimal = featureFlags.GetEnableParameterizedDecimal(); EnableDataErasure = featureFlags.GetEnableDataErasure(); } diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.h b/ydb/core/tx/schemeshard/schemeshard_impl.h index cba9966da885..bb0559fd6cb3 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.h +++ b/ydb/core/tx/schemeshard/schemeshard_impl.h @@ -334,16 +334,13 @@ class TSchemeShard bool EnableAlterDatabaseCreateHiveFirst = false; bool EnablePQConfigTransactionsAtSchemeShard = false; bool EnableStatistics = false; - bool EnableTablePgTypes = false; bool EnableServerlessExclusiveDynamicNodes = false; bool EnableAddColumsWithDefaults = false; bool EnableReplaceIfExistsForExternalEntities = false; bool EnableTempTables = false; - bool EnableTableDatetime64 = false; bool EnableResourcePoolsOnServerless = false; bool EnableVectorIndex = false; bool EnableExternalDataSourcesOnServerless = false; - bool EnableParameterizedDecimal = false; bool EnableDataErasure = false; TShardDeleter ShardDeleter; From d98dc43df59fb63ee8be3252e7b8ab7c3a0b7db4 Mon Sep 17 00:00:00 2001 From: azevaykin Date: Wed, 16 Apr 2025 17:42:40 +0300 Subject: [PATCH 2/2] Copy table should not check feature flags for columns types (#17290) --- .../schemeshard__operation_copy_table.cpp | 8 +++-- .../ut_base/ut_table_decimal_types.cpp | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp index 13a2e0cd3a5f..209a6e6e9619 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp @@ -578,10 +578,12 @@ class TCopyTable: public TSubOperation { const NScheme::TTypeRegistry* typeRegistry = AppData()->TypeRegistry; const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); + // Copy table should not check feature flags for columns types. + // If the types in original table are created then they should be allowed in destination table. const TTableInfo::TCreateAlterDataFeatureFlags featureFlags = { - .EnableTablePgTypes = AppData()->FeatureFlags.GetEnableTablePgTypes(), - .EnableTableDatetime64 = AppData()->FeatureFlags.GetEnableTableDatetime64(), - .EnableParameterizedDecimal = AppData()->FeatureFlags.GetEnableParameterizedDecimal(), + .EnableTablePgTypes = true, + .EnableTableDatetime64 = true, + .EnableParameterizedDecimal = true, }; TTableInfo::TAlterDataPtr alterData = TTableInfo::CreateAlterData(nullptr, schema, *typeRegistry, limits, *domainInfo, featureFlags, errStr, LocalSequences); diff --git a/ydb/core/tx/schemeshard/ut_base/ut_table_decimal_types.cpp b/ydb/core/tx/schemeshard/ut_base/ut_table_decimal_types.cpp index a05ce0d9d593..6f9ce1c7e497 100644 --- a/ydb/core/tx/schemeshard/ut_base/ut_table_decimal_types.cpp +++ b/ydb/core/tx/schemeshard/ut_base/ut_table_decimal_types.cpp @@ -107,6 +107,39 @@ Y_UNIT_TEST_SUITE(TSchemeShardDecimalTypesInTables) { }); } + Y_UNIT_TEST(CopyTableShouldNotFailOnDisabledFeatureFlag) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableParameterizedDecimal(true)); + ui64 txId = 100; + + AsyncCreateTable(runtime, ++txId, "/MyRoot", R"_( + Name: "Table1" + Columns { Name: "key" Type: "Decimal(35,6)" } + Columns { Name: "value" Type: "Decimal(35,6)" } + KeyColumnNames: ["key"] + )_"); + TestModificationResults(runtime, txId, {TExpectedResult(NKikimrScheme::StatusAccepted)}); + env.TestWaitNotification(runtime, txId); + + TestDescribeResult(DescribePath(runtime, "/MyRoot/Table1"), { + NLs::PathExist, + NLs::Finished, + NLs::CheckColumnType(0, "Decimal(35,6)") + }); + + runtime.GetAppData().FeatureFlags.SetEnableParameterizedDecimal(false); + + AsyncCopyTable(runtime, ++txId, "/MyRoot", "Copy1", "/MyRoot/Table1"); + TestModificationResults(runtime, txId, {TExpectedResult(NKikimrScheme::StatusAccepted)}); + env.TestWaitNotification(runtime, txId); + + TestDescribeResult(DescribePath(runtime, "/MyRoot/Copy1"), { + NLs::PathExist, + NLs::Finished, + NLs::CheckColumnType(0, "Decimal(35,6)") + }); + } + Y_UNIT_TEST(CreateWithWrongParameters) { TTestBasicRuntime runtime; TTestEnv env(runtime, TTestEnvOptions().EnableParameterizedDecimal(true));