diff --git a/ydb/core/kqp/ut/olap/tiering_ut.cpp b/ydb/core/kqp/ut/olap/tiering_ut.cpp index f10684a2af30..b9cceba93738 100644 --- a/ydb/core/kqp/ut/olap/tiering_ut.cpp +++ b/ydb/core/kqp/ut/olap/tiering_ut.cpp @@ -92,6 +92,70 @@ Y_UNIT_TEST_SUITE(KqpOlapTiering) { << " after=" << GetUint64(rows[0].at("RawBytes"))); } } + + Y_UNIT_TEST(TieringRuleValidation) { + auto csController = NYDBTest::TControllers::RegisterCSControllerGuard(); + + TKikimrSettings runnerSettings; + runnerSettings.WithSampleTables = false; + TTestHelper testHelper(runnerSettings); + TLocalHelper localHelper(testHelper.GetKikimr()); + NYdb::NTable::TTableClient tableClient = testHelper.GetKikimr().GetTableClient(); + Tests::NCommon::TLoggerInit(testHelper.GetKikimr()).Initialize(); + Singleton()->SetSecretKey("fakeSecret"); + + localHelper.CreateTestOlapTable(); + testHelper.CreateTier("tier1"); + + { + const TString query = R"( + CREATE OBJECT IF NOT EXISTS empty_tiering_rule (TYPE TIERING_RULE) + WITH (defaultColumn = timestamp, description = `{"rules": []}`))"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + { + const TString query = R"( + CREATE OBJECT IF NOT EXISTS empty_default_column (TYPE TIERING_RULE) + WITH (defaultColumn = ``, description = `{"rules": [{ "tierName" : "tier1", "durationForEvict" : "10d" }]}`))"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + { + const TString query = R"( + CREATE OBJECT IF NOT EXISTS no_default_column (TYPE TIERING_RULE) + WITH (description = `{"rules": [{ "tierName" : "tier1", "durationForEvict" : "10d" }]}`))"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + const TString correctTieringRule = testHelper.CreateTieringRule("tier1", "timestamp"); + { + const TString query = "ALTER OBJECT " + correctTieringRule + R"( (TYPE TIERING_RULE) SET description `{"rules": []}`)"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + { + const TString query = "ALTER OBJECT " + correctTieringRule + R"( (TYPE TIERING_RULE) SET description `{"rules": []}`)"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + { + const TString query = "ALTER OBJECT " + correctTieringRule + R"( (TYPE TIERING_RULE) SET defaultColumn ``)"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + + { + const TString query = "ALTER OBJECT " + correctTieringRule + R"( (TYPE TIERING_RULE) RESET defaultColumn)"; + auto result = testHelper.GetSession().ExecuteSchemeQuery(query).GetValueSync(); + UNIT_ASSERT_VALUES_UNEQUAL(result.GetStatus(), NYdb::EStatus::SUCCESS); + } + } } } // namespace NKikimr::NKqp diff --git a/ydb/core/tx/columnshard/engines/scheme/tiering/tier_info.h b/ydb/core/tx/columnshard/engines/scheme/tiering/tier_info.h index c65cb1703ad0..1f5411e8e03c 100644 --- a/ydb/core/tx/columnshard/engines/scheme/tiering/tier_info.h +++ b/ydb/core/tx/columnshard/engines/scheme/tiering/tier_info.h @@ -109,7 +109,7 @@ class TTiering { using TTiersMap = THashMap>; TTiersMap TierByName; TSet OrderedTiers; - TString TTLColumnName; + std::optional TTLColumnName; public: class TTieringContext { @@ -174,9 +174,14 @@ class TTiering { [[nodiscard]] bool Add(const std::shared_ptr& tier) { AFL_VERIFY(tier); if (!TTLColumnName) { + if (tier->GetEvictColumnName().Empty()) { + AFL_ERROR(NKikimrServices::TX_COLUMNSHARD)("problem", "empty_evict_column_name"); + return false; + } TTLColumnName = tier->GetEvictColumnName(); - } else if (TTLColumnName != tier->GetEvictColumnName()) { - AFL_ERROR(NKikimrServices::TX_COLUMNSHARD)("problem", "incorrect_tiering_metadata")("column_before", TTLColumnName)("column_new", tier->GetEvictColumnName()); + } else if (*TTLColumnName != tier->GetEvictColumnName()) { + AFL_ERROR(NKikimrServices::TX_COLUMNSHARD)("problem", "incorrect_tiering_metadata")("column_before", *TTLColumnName) + ("column_new", tier->GetEvictColumnName()); return false; } @@ -194,13 +199,9 @@ class TTiering { return {}; } - const TString& GetTtlColumn() const { - AFL_VERIFY(TTLColumnName); - return TTLColumnName; - } - const TString& GetEvictColumnName() const { - return TTLColumnName; + AFL_VERIFY(TTLColumnName); + return *TTLColumnName; } TString GetDebugString() const { diff --git a/ydb/core/tx/columnshard/engines/storage/actualizer/tiering/tiering.cpp b/ydb/core/tx/columnshard/engines/storage/actualizer/tiering/tiering.cpp index 87fecc13e472..55882102eee9 100644 --- a/ydb/core/tx/columnshard/engines/storage/actualizer/tiering/tiering.cpp +++ b/ydb/core/tx/columnshard/engines/storage/actualizer/tiering/tiering.cpp @@ -174,7 +174,7 @@ void TTieringActualizer::DoExtractTasks(TTieringProcessContext& tasksContext, co void TTieringActualizer::Refresh(const std::optional& info, const TAddExternalContext& externalContext) { Tiering = info; if (Tiering) { - TieringColumnId = VersionedIndex.GetLastSchema()->GetColumnId(Tiering->GetTtlColumn()); + TieringColumnId = VersionedIndex.GetLastSchema()->GetColumnId(Tiering->GetEvictColumnName()); } else { TieringColumnId = {}; } diff --git a/ydb/core/tx/tiering/manager.cpp b/ydb/core/tx/tiering/manager.cpp index 85dd6d60c10b..57462d745d3a 100644 --- a/ydb/core/tx/tiering/manager.cpp +++ b/ydb/core/tx/tiering/manager.cpp @@ -199,19 +199,18 @@ THashMap TTiersManager::GetTiering() const { Y_ABORT_UNLESS(snapshotPtr); auto& tierConfigs = snapshotPtr->GetTierConfigs(); for (auto&& i : PathIdTiering) { - auto* tiering = snapshotPtr->GetTieringById(i.second); - if (tiering) { + auto* tieringRule = snapshotPtr->GetTieringById(i.second); + if (tieringRule) { AFL_DEBUG(NKikimrServices::TX_COLUMNSHARD)("path_id", i.first)("tiering_name", i.second)("event", "activation"); - result.emplace(i.first, tiering->BuildOlapTiers()); - for (auto& [pathId, pathTiering] : result) { - for (auto& [name, tier] : pathTiering.GetTierByName()) { - AFL_VERIFY(name != NOlap::NTiering::NCommon::DeleteTierName); - auto it = tierConfigs.find(name); - if (it != tierConfigs.end()) { - tier->SetSerializer(NTiers::ConvertCompression(it->second.GetCompression())); - } + NOlap::TTiering tiering = tieringRule->BuildOlapTiers(); + for (auto& [name, tier] : tiering.GetTierByName()) { + AFL_VERIFY(name != NOlap::NTiering::NCommon::DeleteTierName); + auto it = tierConfigs.find(name); + if (it != tierConfigs.end()) { + tier->SetSerializer(NTiers::ConvertCompression(it->second.GetCompression())); } } + result.emplace(i.first, std::move(tiering)); } else { AFL_ERROR(NKikimrServices::TX_COLUMNSHARD)("path_id", i.first)("tiering_name", i.second)("event", "not_found"); } diff --git a/ydb/core/tx/tiering/rule/manager.cpp b/ydb/core/tx/tiering/rule/manager.cpp index 99f8ad1177a2..321bfec34593 100644 --- a/ydb/core/tx/tiering/rule/manager.cpp +++ b/ydb/core/tx/tiering/rule/manager.cpp @@ -21,6 +21,9 @@ NMetadata::NModifications::TOperationParsingResult TTieringRulesManager::DoBuild { auto fValue = settings.GetFeaturesExtractor().Extract(TTieringRule::TDecoder::DefaultColumn); if (fValue) { + if (fValue->Empty()) { + return TConclusionStatus::Fail("defaultColumn cannot be empty"); + } result.SetColumn(TTieringRule::TDecoder::DefaultColumn, NMetadata::NInternal::TYDBValue::Utf8(*fValue)); } } diff --git a/ydb/core/tx/tiering/rule/object.cpp b/ydb/core/tx/tiering/rule/object.cpp index 59d42bdb4c8e..a596b56890ca 100644 --- a/ydb/core/tx/tiering/rule/object.cpp +++ b/ydb/core/tx/tiering/rule/object.cpp @@ -30,6 +30,10 @@ bool TTieringRule::DeserializeDescriptionFromJson(const NJson::TJsonValue& jsonI if (!jsonInfo["rules"].GetArrayPointer(&rules)) { return false; } + if (rules->empty()) { + AFL_INFO(NKikimrServices::TX_COLUMNSHARD)("event", "tiering_rule_deserialization_failed")("reason", "empty_rules"); + return false; + } for (auto&& i : *rules) { TTieringInterval interval; if (!interval.DeserializeFromJson(i)) { @@ -61,6 +65,9 @@ bool TTieringRule::DeserializeFromRecord(const TDecoder& decoder, const Ydb::Val if (!decoder.Read(decoder.GetDefaultColumnIdx(), DefaultColumn, r)) { return false; } + if (DefaultColumn.Empty()) { + return false; + } NJson::TJsonValue jsonDescription; if (!decoder.ReadJson(decoder.GetDescriptionIdx(), jsonDescription, r)) { return false; @@ -72,6 +79,7 @@ bool TTieringRule::DeserializeFromRecord(const TDecoder& decoder, const Ydb::Val } NKikimr::NOlap::TTiering TTieringRule::BuildOlapTiers() const { + AFL_VERIFY(!Intervals.empty()); NOlap::TTiering result; for (auto&& r : Intervals) { AFL_VERIFY(result.Add(std::make_shared(r.GetTierName(), r.GetDurationForEvict(), GetDefaultColumn())));