From 38a20d2e376793d0789c309226abf2e5f264404e Mon Sep 17 00:00:00 2001 From: Alexander Avdonkin Date: Tue, 10 Sep 2024 09:28:12 +0000 Subject: [PATCH 1/5] Sparsed columns are disabled by default and can be optionally enabled --- ydb/core/kqp/ut/olap/sparsed_ut.cpp | 7 ++++++- ydb/core/kqp/ut/olap/write_ut.cpp | 3 +++ ydb/core/protos/feature_flags.proto | 1 + .../olap/operations/alter/standalone/update.cpp | 9 +++++++++ .../tx/schemeshard/olap/operations/alter_store.cpp | 12 ++++++++++++ ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp | 11 +++++++++++ ydb/core/tx/schemeshard/ut_ttl/ut_ttl.cpp | 8 ++++++++ 7 files changed, 50 insertions(+), 1 deletion(-) diff --git a/ydb/core/kqp/ut/olap/sparsed_ut.cpp b/ydb/core/kqp/ut/olap/sparsed_ut.cpp index 73b75f2cc53f..b9c9ad76206c 100644 --- a/ydb/core/kqp/ut/olap/sparsed_ut.cpp +++ b/ydb/core/kqp/ut/olap/sparsed_ut.cpp @@ -24,11 +24,16 @@ Y_UNIT_TEST_SUITE(KqpOlapSparsed) { const TVector FIELD_NAMES{"utf", "int", "uint", "float", "double"}; public: TSparsedDataTest(const TString& storeName) - : Kikimr(Settings) + : Kikimr(GetKikimrSettings()) , CSController(NKikimr::NYDBTest::TControllers::RegisterCSControllerGuard()) , StoreName(storeName) { + } + TKikimrSettings GetKikimrSettings() { + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableSparsedColumns(true); + return TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); } ui32 GetCount() const { diff --git a/ydb/core/kqp/ut/olap/write_ut.cpp b/ydb/core/kqp/ut/olap/write_ut.cpp index 8d9751f28193..ad9766fc2b9c 100644 --- a/ydb/core/kqp/ut/olap/write_ut.cpp +++ b/ydb/core/kqp/ut/olap/write_ut.cpp @@ -133,6 +133,9 @@ Y_UNIT_TEST_SUITE(KqpOlapWrite) { Y_UNIT_TEST(DefaultValues) { auto settings = TKikimrSettings().SetWithSampleTables(false); + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableSparsedColumns(true); + settings.SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); TTypedLocalHelper helper("Utf8", kikimr); diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index 9570be7581aa..2f950700d1fa 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -160,4 +160,5 @@ message TFeatureFlags { optional bool EnableMetadataObjectsOnServerless = 141 [default = true]; optional bool EnableOlapCompression = 142 [default = false]; optional bool EnableExternalDataSourcesOnServerless = 143 [default = true]; + optional bool EnableSparsedColumns = 144 [default = false]; } diff --git a/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp b/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp index cfb1367a224f..8404fb1f51ef 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp @@ -1,6 +1,7 @@ #include "update.h" #include #include +#include namespace NKikimr::NSchemeShard::NOlap::NAlter { @@ -51,6 +52,14 @@ NKikimr::TConclusionStatus TStandaloneSchemaUpdate::DoInitializeImpl(const TUpda } } + if (!AppData()->FeatureFlags.GetEnableSparsedColumns()) { + for (auto& [_, column]: targetSchema.GetColumns().GetColumns()) { + if (column.GetDefaultValue().GetValue() || (column.GetAccessorConstructor().GetClassName() == NKikimr::NArrow::NAccessor::TGlobalConst::SparsedDataAccessorName)) { + return TConclusionStatus::Fail("schema update error: sparsed columns are disabled"); + } + } + } + auto description = originalTable.GetTableInfoVerified().Description; targetSchema.Serialize(*description.MutableSchema()); auto ttl = originalTable.GetTableTTLOptional() ? *originalTable.GetTableTTLOptional() : TOlapTTL(); diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp index d61f84b81209..98a4b6b8d1c7 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include "checks.h" @@ -525,6 +526,17 @@ class TAlterOlapStore: public TSubOperation { return result; } + if (!AppData()->FeatureFlags.GetEnableSparsedColumns()) { + for (auto& [_, preset]: alterData->SchemaPresets) { + for (auto& [_, column]: preset.GetColumns().GetColumns()) { + if (column.GetDefaultValue().GetValue() || (column.GetAccessorConstructor().GetClassName() == NKikimr::NArrow::NAccessor::TGlobalConst::SparsedDataAccessorName)) { + result->SetError(NKikimrScheme::StatusSchemeError,"schema update error: sparsed columns are disabled"); + return result; + } + } + } + } + auto domainInfo = parentPath.DomainInfo(); const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); diff --git a/ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp b/ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp index 90a14c747f8a..1a6a85e46e27 100644 --- a/ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp +++ b/ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp @@ -557,6 +557,17 @@ Y_UNIT_TEST_SUITE(TOlap) { } } )", {NKikimrScheme::StatusAccepted}); + + env.TestWaitNotification(runtime, txId); + TestAlterOlapStore(runtime, ++txId, "/MyRoot", R"( + Name: "OlapStore" + AlterSchemaPresets { + Name: "default" + AlterSchema { + AlterColumns { Name: "comment" DefaultValue: "10" } + } + } + )", {NKikimrScheme::StatusSchemeError}); } Y_UNIT_TEST(AlterTtl) { diff --git a/ydb/core/tx/schemeshard/ut_ttl/ut_ttl.cpp b/ydb/core/tx/schemeshard/ut_ttl/ut_ttl.cpp index ce57f14992b3..1accb55c269b 100644 --- a/ydb/core/tx/schemeshard/ut_ttl/ut_ttl.cpp +++ b/ydb/core/tx/schemeshard/ut_ttl/ut_ttl.cpp @@ -1150,6 +1150,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardColumnTableTTL) { Columns { Name: "key" Type: "Uint64" NotNull: true } Columns { Name: "modified_at" Type: "Timestamp" } Columns { Name: "saved_at" Type: "Datetime" } + Columns { Name: "data" Type: "Utf8" } KeyColumnNames: ["key"] } )"); @@ -1206,6 +1207,13 @@ Y_UNIT_TEST_SUITE(TSchemeShardColumnTableTTL) { } } ); + TestAlterColumnTable(runtime, ++txId, "/MyRoot", R"( + Name: "TTLEnabledTable" + AlterSchema { + AlterColumns {Name: "data" DefaultValue: "10"} + } + )", {NKikimrScheme::StatusSchemeError}); + env.TestWaitNotification(runtime, txId); } Y_UNIT_TEST(AlterColumnTable_Negative) { From 7571462d704c412696e962df811faa9c48fbb4b1 Mon Sep 17 00:00:00 2001 From: Alexander Avdonkin Date: Wed, 11 Sep 2024 12:13:52 +0000 Subject: [PATCH 2/5] Sparsed columns are enabled in all tests by default; sparsed column check moved to TColumnTableUpdate --- ydb/core/kqp/ut/common/kqp_ut_common.h | 3 +++ ydb/core/kqp/ut/olap/sparsed_ut.cpp | 8 +------- .../olap/operations/alter/common/update.h | 12 ++++++++++++ .../olap/operations/alter/standalone/update.cpp | 9 ++------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/ydb/core/kqp/ut/common/kqp_ut_common.h b/ydb/core/kqp/ut/common/kqp_ut_common.h index 9e86aa3645e5..08d79d9603f6 100644 --- a/ydb/core/kqp/ut/common/kqp_ut_common.h +++ b/ydb/core/kqp/ut/common/kqp_ut_common.h @@ -97,6 +97,9 @@ struct TKikimrSettings: public TTestFeatureFlagsHolder { exchangerSettings->SetStartDelayMs(10); exchangerSettings->SetMaxDelayMs(10); AppConfig.MutableColumnShardConfig()->SetDisabledOnSchemeShard(false); + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableSparsedColumns(true); + SetFeatureFlags(featureFlags); } TKikimrSettings& SetAppConfig(const NKikimrConfig::TAppConfig& value) { AppConfig = value; return *this; } diff --git a/ydb/core/kqp/ut/olap/sparsed_ut.cpp b/ydb/core/kqp/ut/olap/sparsed_ut.cpp index b9c9ad76206c..4e4ade7b0e69 100644 --- a/ydb/core/kqp/ut/olap/sparsed_ut.cpp +++ b/ydb/core/kqp/ut/olap/sparsed_ut.cpp @@ -24,18 +24,12 @@ Y_UNIT_TEST_SUITE(KqpOlapSparsed) { const TVector FIELD_NAMES{"utf", "int", "uint", "float", "double"}; public: TSparsedDataTest(const TString& storeName) - : Kikimr(GetKikimrSettings()) + : Kikimr(Settings) , CSController(NKikimr::NYDBTest::TControllers::RegisterCSControllerGuard()) , StoreName(storeName) { } - TKikimrSettings GetKikimrSettings() { - NKikimrConfig::TFeatureFlags featureFlags; - featureFlags.SetEnableSparsedColumns(true); - return TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); - } - ui32 GetCount() const { auto selectQuery = TString(R"( SELECT diff --git a/ydb/core/tx/schemeshard/olap/operations/alter/common/update.h b/ydb/core/tx/schemeshard/olap/operations/alter/common/update.h index 8375e6fa0e4f..013c5601780e 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter/common/update.h +++ b/ydb/core/tx/schemeshard/olap/operations/alter/common/update.h @@ -2,6 +2,7 @@ #include #include #include +#include namespace NKikimr::NSchemeShard::NOlap::NAlter { @@ -65,6 +66,17 @@ class TColumnTableUpdate: public ISSEntityUpdate { return result; } + bool CheckTargetSchema(const TOlapSchema& targetSchema) { + if (!AppData()->FeatureFlags.GetEnableSparsedColumns()) { + for (auto& [_, column]: targetSchema.GetColumns().GetColumns()) { + if (column.GetDefaultValue().GetValue() || (column.GetAccessorConstructor().GetClassName() == NKikimr::NArrow::NAccessor::TGlobalConst::SparsedDataAccessorName)) { + return false; + } + } + } + return true; + } + public: }; diff --git a/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp b/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp index 8404fb1f51ef..a442ca80392f 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter/standalone/update.cpp @@ -52,14 +52,9 @@ NKikimr::TConclusionStatus TStandaloneSchemaUpdate::DoInitializeImpl(const TUpda } } - if (!AppData()->FeatureFlags.GetEnableSparsedColumns()) { - for (auto& [_, column]: targetSchema.GetColumns().GetColumns()) { - if (column.GetDefaultValue().GetValue() || (column.GetAccessorConstructor().GetClassName() == NKikimr::NArrow::NAccessor::TGlobalConst::SparsedDataAccessorName)) { - return TConclusionStatus::Fail("schema update error: sparsed columns are disabled"); - } - } + if (!CheckTargetSchema(targetSchema)) { + return TConclusionStatus::Fail("schema update error: sparsed columns are disabled"); } - auto description = originalTable.GetTableInfoVerified().Description; targetSchema.Serialize(*description.MutableSchema()); auto ttl = originalTable.GetTableTTLOptional() ? *originalTable.GetTableTTLOptional() : TOlapTTL(); From fbd75ff65c7b3a1d1bfc01490d1ed57cee8a7a8b Mon Sep 17 00:00:00 2001 From: Alexander Avdonkin Date: Wed, 11 Sep 2024 12:36:14 +0000 Subject: [PATCH 3/5] Restored empty line --- ydb/core/kqp/ut/olap/sparsed_ut.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/ydb/core/kqp/ut/olap/sparsed_ut.cpp b/ydb/core/kqp/ut/olap/sparsed_ut.cpp index 4e4ade7b0e69..73b75f2cc53f 100644 --- a/ydb/core/kqp/ut/olap/sparsed_ut.cpp +++ b/ydb/core/kqp/ut/olap/sparsed_ut.cpp @@ -28,6 +28,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSparsed) { , CSController(NKikimr::NYDBTest::TControllers::RegisterCSControllerGuard()) , StoreName(storeName) { + } ui32 GetCount() const { From 9bd7a6b0277d3ec3bab8cbecbcc684be37435066 Mon Sep 17 00:00:00 2001 From: Alexander Avdonkin Date: Wed, 11 Sep 2024 16:22:01 +0000 Subject: [PATCH 4/5] Removed unnecessary code --- ydb/core/kqp/ut/olap/write_ut.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/ydb/core/kqp/ut/olap/write_ut.cpp b/ydb/core/kqp/ut/olap/write_ut.cpp index ad9766fc2b9c..8d9751f28193 100644 --- a/ydb/core/kqp/ut/olap/write_ut.cpp +++ b/ydb/core/kqp/ut/olap/write_ut.cpp @@ -133,9 +133,6 @@ Y_UNIT_TEST_SUITE(KqpOlapWrite) { Y_UNIT_TEST(DefaultValues) { auto settings = TKikimrSettings().SetWithSampleTables(false); - NKikimrConfig::TFeatureFlags featureFlags; - featureFlags.SetEnableSparsedColumns(true); - settings.SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); TTypedLocalHelper helper("Utf8", kikimr); From fdf216d66461c63bb63cc86c65a18720f17334ba Mon Sep 17 00:00:00 2001 From: Alexander Avdonkin Date: Thu, 12 Sep 2024 08:29:48 +0000 Subject: [PATCH 5/5] Do not replace whole FeatureFlags --- ydb/core/kqp/ut/common/kqp_ut_common.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ydb/core/kqp/ut/common/kqp_ut_common.h b/ydb/core/kqp/ut/common/kqp_ut_common.h index 08d79d9603f6..552687f47857 100644 --- a/ydb/core/kqp/ut/common/kqp_ut_common.h +++ b/ydb/core/kqp/ut/common/kqp_ut_common.h @@ -97,9 +97,7 @@ struct TKikimrSettings: public TTestFeatureFlagsHolder { exchangerSettings->SetStartDelayMs(10); exchangerSettings->SetMaxDelayMs(10); AppConfig.MutableColumnShardConfig()->SetDisabledOnSchemeShard(false); - NKikimrConfig::TFeatureFlags featureFlags; - featureFlags.SetEnableSparsedColumns(true); - SetFeatureFlags(featureFlags); + FeatureFlags.SetEnableSparsedColumns(true); } TKikimrSettings& SetAppConfig(const NKikimrConfig::TAppConfig& value) { AppConfig = value; return *this; }