From 68abb6445c86447265695823a06f33fae9a0ebeb Mon Sep 17 00:00:00 2001 From: vlad-gogov Date: Wed, 4 Sep 2024 11:57:49 +0000 Subject: [PATCH 1/4] add feature flag: enable olap compression --- ydb/core/kqp/ut/olap/sys_view_ut.cpp | 10 ++++++---- ydb/core/protos/feature_flags.proto | 1 + ydb/core/testlib/basics/feature_flags.h | 19 ++++++++++--------- .../tx/schemeshard/olap/columns/update.cpp | 5 +++++ 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/ydb/core/kqp/ut/olap/sys_view_ut.cpp b/ydb/core/kqp/ut/olap/sys_view_ut.cpp index 27820452542e..4891366b93df 100644 --- a/ydb/core/kqp/ut/olap/sys_view_ut.cpp +++ b/ydb/core/kqp/ut/olap/sys_view_ut.cpp @@ -131,8 +131,9 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) { const ui32 rowsCount = 800000; const ui32 groupsCount = 512; { - auto settings = TKikimrSettings() - .SetWithSampleTables(false); + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableCompression(true); + auto settings = TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); TTypedLocalHelper helper("Utf8", kikimr); @@ -186,8 +187,9 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) { ui64 rawBytesPK1; ui64 bytesPK1; auto csController = NYDBTest::TControllers::RegisterCSControllerGuard(); - auto settings = TKikimrSettings() - .SetWithSampleTables(false); + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableCompression(true); + auto settings = TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); TTypedLocalHelper helper("", kikimr, "olapTable", "olapStore"); diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index fc7346a8c3cb..3ce8491bb6c0 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -157,4 +157,5 @@ message TFeatureFlags { optional bool EnableAlterShardingInColumnShard = 138 [default = false]; optional bool EnablePgSyntax = 139 [default = true]; optional bool EnableTieringInColumnShard = 140 [default = false]; + optional bool EnableOlapCompression = 141 [default = false]; } diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index 1dbdab825a3c..b77296c7b8b3 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -9,13 +9,13 @@ class TTestFeatureFlagsHolder { public: TFeatureFlags FeatureFlags; - #define FEATURE_FLAG_SETTER(name) \ - TDerived& Set##name(std::optional value) { \ - if (value) { \ - FeatureFlags.Set##name(*value); \ - } \ - return *static_cast(this); \ - } +#define FEATURE_FLAG_SETTER(name) \ + TDerived& Set##name(std::optional value) { \ + if (value) { \ + FeatureFlags.Set##name(*value); \ + } \ + return *static_cast(this); \ + } FEATURE_FLAG_SETTER(AllowYdbRequestsWithoutDatabase) FEATURE_FLAG_SETTER(EnableSystemViews) @@ -66,8 +66,9 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableGranularTimecast) FEATURE_FLAG_SETTER(EnablePgSyntax) FEATURE_FLAG_SETTER(EnableTieringInColumnShard) + FEATURE_FLAG_SETTER(EnableOlapCompression) - #undef FEATURE_FLAG_SETTER +#undef FEATURE_FLAG_SETTER }; -} // NKikimr +} diff --git a/ydb/core/tx/schemeshard/olap/columns/update.cpp b/ydb/core/tx/schemeshard/olap/columns/update.cpp index c66da237c712..b92c24a65b2f 100644 --- a/ydb/core/tx/schemeshard/olap/columns/update.cpp +++ b/ydb/core/tx/schemeshard/olap/columns/update.cpp @@ -271,6 +271,11 @@ namespace NKikimr::NSchemeShard { TSet alterColumnNames; for (auto& columnSchemaDiff : alterRequest.GetAlterColumns()) { + if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression() && columnSchemaDiff.HasSerializer()) { + errors.AddError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); + return false; + } + TOlapColumnDiff columnDiff; if (!columnDiff.ParseFromRequest(columnSchemaDiff, errors)) { return false; From 9c0ede66230035f9ea18480f8d730a47c40bcbea Mon Sep 17 00:00:00 2001 From: vlad-gogov Date: Wed, 4 Sep 2024 12:08:11 +0000 Subject: [PATCH 2/4] fix test --- ydb/core/kqp/ut/olap/sys_view_ut.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ydb/core/kqp/ut/olap/sys_view_ut.cpp b/ydb/core/kqp/ut/olap/sys_view_ut.cpp index 4891366b93df..5896b49aeb97 100644 --- a/ydb/core/kqp/ut/olap/sys_view_ut.cpp +++ b/ydb/core/kqp/ut/olap/sys_view_ut.cpp @@ -132,7 +132,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) { const ui32 groupsCount = 512; { NKikimrConfig::TFeatureFlags featureFlags; - featureFlags.SetEnableCompression(true); + featureFlags.SetEnableOlapCompression(true); auto settings = TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); @@ -188,7 +188,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) { ui64 bytesPK1; auto csController = NYDBTest::TControllers::RegisterCSControllerGuard(); NKikimrConfig::TFeatureFlags featureFlags; - featureFlags.SetEnableCompression(true); + featureFlags.SetEnableOlapCompression(true); auto settings = TKikimrSettings().SetWithSampleTables(false).SetFeatureFlags(featureFlags); TKikimrRunner kikimr(settings); Tests::NCommon::TLoggerInit(kikimr).Initialize(); From 7c37c3f3423af446722b50a7d4987a8024ccf953 Mon Sep 17 00:00:00 2001 From: vlad-gogov Date: Wed, 4 Sep 2024 13:40:20 +0000 Subject: [PATCH 3/4] check moved above --- ydb/core/tx/schemeshard/olap/columns/update.cpp | 5 ----- .../tx/schemeshard/olap/operations/alter_store.cpp | 11 +++++++++++ .../tx/schemeshard/olap/operations/alter_table.cpp | 9 +++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/ydb/core/tx/schemeshard/olap/columns/update.cpp b/ydb/core/tx/schemeshard/olap/columns/update.cpp index b92c24a65b2f..c66da237c712 100644 --- a/ydb/core/tx/schemeshard/olap/columns/update.cpp +++ b/ydb/core/tx/schemeshard/olap/columns/update.cpp @@ -271,11 +271,6 @@ namespace NKikimr::NSchemeShard { TSet alterColumnNames; for (auto& columnSchemaDiff : alterRequest.GetAlterColumns()) { - if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression() && columnSchemaDiff.HasSerializer()) { - errors.AddError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); - return false; - } - TOlapColumnDiff columnDiff; if (!columnDiff.ParseFromRequest(columnSchemaDiff, errors)) { return false; diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp index 446b43017821..1cb3ee11c905 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp @@ -453,6 +453,17 @@ class TAlterOlapStore: public TSubOperation { auto result = MakeHolder(NKikimrScheme::StatusAccepted, ui64(OperationId.GetTxId()), ui64(ssId)); + if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression()) { + for (const auto& alterSchema : alter.GetAlterSchemaPresets()) { + for (const auto& alterColumn : alterSchema.GetAlterSchema().GetAlterColumns()) { + if (alterColumn.HasSerializer()) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); + return result; + } + } + } + } + if (!alter.HasName()) { result->SetError(NKikimrScheme::StatusInvalidParameter, "No store name in Alter"); return result; diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp index 4fb76b4a75a0..8b2a50d0629a 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp @@ -270,6 +270,15 @@ class TAlterColumnTable: public TSubOperation { result->SetError(NKikimrScheme::StatusPreconditionFailed, "Alter sharding is disabled for OLAP tables"); return result; } + + if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression()) { + for (const auto& alterColumn : Transaction.GetAlterColumnTable().GetAlterSchema().GetAlterColumns()) { + if (alterColumn.HasSerializer()) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); + return result; + } + } + } const bool hasTiering = Transaction.HasAlterColumnTable() && Transaction.GetAlterColumnTable().HasAlterTtlSettings() && Transaction.GetAlterColumnTable().GetAlterTtlSettings().HasUseTiering(); From 6d21224509f18478418d043295bc8c9fd7ef1b58 Mon Sep 17 00:00:00 2001 From: vlad-gogov Date: Fri, 6 Sep 2024 11:19:53 +0300 Subject: [PATCH 4/4] refactoring --- ydb/core/testlib/basics/feature_flags.h | 18 ++++++------ .../olap/operations/alter_store.cpp | 28 +++++++++++-------- .../olap/operations/alter_table.cpp | 19 ++++++++----- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index b77296c7b8b3..9adabb7f5383 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -9,13 +9,13 @@ class TTestFeatureFlagsHolder { public: TFeatureFlags FeatureFlags; -#define FEATURE_FLAG_SETTER(name) \ - TDerived& Set##name(std::optional value) { \ - if (value) { \ - FeatureFlags.Set##name(*value); \ - } \ - return *static_cast(this); \ - } + #define FEATURE_FLAG_SETTER(name) \ + TDerived& Set##name(std::optional value) { \ + if (value) { \ + FeatureFlags.Set##name(*value); \ + } \ + return *static_cast(this); \ + } FEATURE_FLAG_SETTER(AllowYdbRequestsWithoutDatabase) FEATURE_FLAG_SETTER(EnableSystemViews) @@ -68,7 +68,7 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableTieringInColumnShard) FEATURE_FLAG_SETTER(EnableOlapCompression) -#undef FEATURE_FLAG_SETTER + #undef FEATURE_FLAG_SETTER }; -} +} // NKikimr diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp index 1cb3ee11c905..5ceeef6e63ff 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter_store.cpp @@ -434,6 +434,18 @@ class TAlterOlapStore: public TSubOperation { } } + bool isAlterCompression() const { + const auto& alter = Transaction.GetAlterColumnStore(); + for (const auto& alterSchema : alter.GetAlterSchemaPresets()) { + for (const auto& alterColumn : alterSchema.GetAlterSchema().GetAlterColumns()) { + if (alterColumn.HasSerializer()) { + return true; + } + } + } + return false; + } + public: using TSubOperation::TSubOperation; @@ -453,22 +465,16 @@ class TAlterOlapStore: public TSubOperation { auto result = MakeHolder(NKikimrScheme::StatusAccepted, ui64(OperationId.GetTxId()), ui64(ssId)); - if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression()) { - for (const auto& alterSchema : alter.GetAlterSchemaPresets()) { - for (const auto& alterColumn : alterSchema.GetAlterSchema().GetAlterColumns()) { - if (alterColumn.HasSerializer()) { - result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); - return result; - } - } - } - } - if (!alter.HasName()) { result->SetError(NKikimrScheme::StatusInvalidParameter, "No store name in Alter"); return result; } + if (!AppDataVerified().FeatureFlags.GetEnableOlapCompression() && isAlterCompression()) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); + return result; + } + TPath path = TPath::Resolve(parentPathStr, context.SS).Dive(name); { TPath::TChecker checks = path.Check(); diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp index 8b2a50d0629a..ec17bd04c5de 100644 --- a/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp +++ b/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp @@ -257,6 +257,15 @@ class TAlterColumnTable: public TSubOperation { } } + bool isAlterCompression() const { + for (const auto& alterColumn : Transaction.GetAlterColumnTable().GetAlterSchema().GetAlterColumns()) { + if (alterColumn.HasSerializer()) { + return true; + } + } + return false; + } + public: using TSubOperation::TSubOperation; @@ -271,13 +280,9 @@ class TAlterColumnTable: public TSubOperation { return result; } - if (HasAppData() && !AppDataVerified().FeatureFlags.GetEnableOlapCompression()) { - for (const auto& alterColumn : Transaction.GetAlterColumnTable().GetAlterSchema().GetAlterColumns()) { - if (alterColumn.HasSerializer()) { - result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); - return result; - } - } + if (!AppDataVerified().FeatureFlags.GetEnableOlapCompression() && isAlterCompression()) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, "Compression is disabled for OLAP tables"); + return result; } const bool hasTiering = Transaction.HasAlterColumnTable() && Transaction.GetAlterColumnTable().HasAlterTtlSettings() &&