Skip to content

Commit

Permalink
Disable ALTER OBJECT for column tables by default (ydb-platform#13299)
Browse files Browse the repository at this point in the history
  • Loading branch information
zverevgeny authored Jan 26, 2025
1 parent d2fb24d commit 9cb9dec
Show file tree
Hide file tree
Showing 12 changed files with 152 additions and 22 deletions.
3 changes: 3 additions & 0 deletions ydb/core/kqp/gateway/behaviour/tablestore/manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ NThreading::TFuture<TTableStoreManager::TYqlConclusionStatus> TTableStoreManager
return NThreading::MakeFuture<TYqlConclusionStatus>(TYqlConclusionStatus::Fail("not implemented"));
case EActivityType::Alter:
try {
if (IsStandalone && !AppData()->ColumnShardConfig.GetAlterObjectEnabled()) {
return NThreading::MakeFuture<TYqlConclusionStatus>(TYqlConclusionStatus::Fail("ALTER OBJECT is disabled for column tables"));
}
auto actionName = settings.GetFeaturesExtractor().Extract("ACTION");
if (!actionName) {
return NThreading::MakeFuture<TYqlConclusionStatus>(TYqlConclusionStatus::Fail("can't find ACTION parameter"));
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/kqp/gateway/behaviour/tablestore/manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace NKikimr::NKqp {

class TTableStoreManager: public NMetadata::NModifications::IOperationsManager {
using TBase = NMetadata::NModifications::IOperationsManager;
bool IsStandalone = false;
const bool IsStandalone;
protected:
NThreading::TFuture<TYqlConclusionStatus> DoModify(const NYql::TObjectSettingsImpl& settings, const ui32 nodeId,
const NMetadata::IClassBehaviour::TPtr& manager, TInternalModificationContext& context) const override;
Expand Down
4 changes: 4 additions & 0 deletions ydb/core/kqp/ut/common/kqp_ut_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ struct TKikimrSettings: public TTestFeatureFlagsHolder<TKikimrSettings> {
TKikimrSettings& SetUseRealThreads(bool value) { UseRealThreads = value; return *this; };
TKikimrSettings& SetEnableForceFollowers(bool value) { EnableForceFollowers = value; return *this; };
TKikimrSettings& SetS3ActorsFactory(std::shared_ptr<NYql::NDq::IS3ActorsFactory> value) { S3ActorsFactory = std::move(value); return *this; };
TKikimrSettings& SetColumnShardAlterObjectEnabled(bool enable) {
AppConfig.MutableColumnShardConfig()->SetAlterObjectEnabled(enable);
return *this;
}
};

class TKikimrRunner {
Expand Down
13 changes: 10 additions & 3 deletions ydb/core/kqp/ut/olap/compression_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ std::pair<ui64, ui64> GetVolumes(

Y_UNIT_TEST_SUITE(KqpOlapCompression) {
Y_UNIT_TEST(DisabledAlterCompression) {
TKikimrSettings settings = TKikimrSettings().SetWithSampleTables(false).SetEnableOlapCompression(false);
auto settings = TKikimrSettings()
.SetEnableOlapCompression(false)
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(settings);
TVector<TTestHelper::TColumnSchema> schema = {
TTestHelper::TColumnSchema().SetName("pk_int").SetType(NScheme::NTypeIds::Uint64).SetNullable(false)
Expand All @@ -68,7 +71,9 @@ Y_UNIT_TEST_SUITE(KqpOlapCompression) {
}

Y_UNIT_TEST(OffCompression) {
TKikimrSettings settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(settings);
TVector<TTestHelper::TColumnSchema> schema = {
TTestHelper::TColumnSchema().SetName("pk_int").SetType(NScheme::NTypeIds::Uint64).SetNullable(false)
Expand All @@ -87,7 +92,9 @@ Y_UNIT_TEST_SUITE(KqpOlapCompression) {
}

Y_UNIT_TEST(TestAlterCompressionTableInTableStore) {
TKikimrSettings settings = TKikimrSettings().SetWithSampleTables(false);
TKikimrSettings settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(settings);
TVector<TTestHelper::TColumnSchema> schema = {
TTestHelper::TColumnSchema().SetName("pk_int").SetType(NScheme::NTypeIds::Uint64).SetNullable(false)
Expand Down
4 changes: 3 additions & 1 deletion ydb/core/kqp/ut/olap/indexes_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ Y_UNIT_TEST_SUITE(KqpOlapIndexes) {
}

Y_UNIT_TEST(CountMinSketchIndex) {
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);

auto csController = NYDBTest::TControllers::RegisterCSControllerGuard<NYDBTest::NColumnShard::TController>();
Expand Down
98 changes: 95 additions & 3 deletions ydb/core/kqp/ut/olap/kqp_olap_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,92 @@ Y_UNIT_TEST_SUITE(KqpOlap) {
};
}

Y_UNIT_TEST(AlterObjectDisabled) {
auto settings = TKikimrSettings()
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
TLocalHelper(kikimr).CreateTestOlapTableWithoutStore();

{
//1. QueryService
//1.1 Check that ALTER OBJECT is not working for column tables
auto client = kikimr.GetQueryClient();
const auto result = client.ExecuteQuery(
"ALTER OBJECT `/Root/olapTable` (TYPE TABLE) SET (ACTION=ALTER_COLUMN, NAME=message, `SERIALIZER.CLASS_NAME`=`ARROW_SERIALIZER`, `COMPRESSION.TYPE`=`zstd`, `COMPRESSION.LEVEL`=`4`)",
NYdb::NQuery::TTxControl::BeginTx().CommitTx()
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
//ALTER OBJECT is not supported via QueryService. When supported, this unit test must be revised
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Prepare operations for TABLE objects are not supported", result.GetIssues().ToString());

//1.2 Check that ALTER TABLE is still working for column tables
{
const auto result = client.ExecuteQuery(
"ALTER TABLE `/Root/olapTable` DROP COLUMN message",
NYdb::NQuery::TTxControl::NoTx()
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
{
const auto result = client.ExecuteQuery(
"ALTER TABLE `/Root/olapTable` ADD COLUMN message Text",
NYdb::NQuery::TTxControl::NoTx()
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
{
const auto result = client.ExecuteQuery(
"ALTER TABLE `/Root/olapTable` ALTER FAMILY default SET compression 'LZ4';",
NYdb::NQuery::TTxControl::NoTx()
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
{
const auto result = client.ExecuteQuery(
"ALTER TABLE `/Root/olapTable` set TTL Interval('P1D') on timestamp;",
NYdb::NQuery::TTxControl::NoTx()
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
}
{
//2. TableService
//2.1 Check that ALTER OBJECT is not working for column tables
auto client = kikimr.GetTableClient();
auto session = client.CreateSession().GetValueSync().GetSession();
const auto result = session.ExecuteSchemeQuery(
"ALTER OBJECT `/Root/olapTable` (TYPE TABLE) SET (ACTION=ALTER_COLUMN, NAME=message, `SERIALIZER.CLASS_NAME`=`ARROW_SERIALIZER`, `COMPRESSION.TYPE`=`zstd`, `COMPRESSION.LEVEL`=`4`)"
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), NYdb::EStatus::GENERIC_ERROR);
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Error: ALTER OBJECT is disabled for column tables", result.GetIssues().ToString());
//2.2 Check that ALTER TABLE is still working for column tables
{
const auto result = session.ExecuteSchemeQuery(
"ALTER TABLE `/Root/olapTable` DROP COLUMN message"
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
{
const auto result = session.ExecuteSchemeQuery(
"ALTER TABLE `/Root/olapTable` ADD COLUMN message Text"
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
}
{
//3. YqlScript
//3.1 Check that ALTER OBJECT is not working for column tables
NYdb::NScripting::TScriptingClient client(kikimr.GetDriver());
auto result = client.ExecuteYqlScript(
"ALTER OBJECT `/Root/olapTable` (TYPE TABLE) SET (ACTION=ALTER_COLUMN, NAME=message, `SERIALIZER.CLASS_NAME`=`ARROW_SERIALIZER`, `COMPRESSION.TYPE`=`zstd`, `COMPRESSION.LEVEL`=`4`)"
).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), NYdb::EStatus::GENERIC_ERROR);
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Error: ALTER OBJECT is disabled for column tables", result.GetIssues().ToString());
//3.2 YqlScript is deprecated, not woth bothering about positive checks
//skipped
}
}

Y_UNIT_TEST(SimpleQueryOlap) {
auto settings = TKikimrSettings()
.SetWithSampleTables(false);
Expand Down Expand Up @@ -2640,7 +2726,9 @@ Y_UNIT_TEST_SUITE(KqpOlap) {
}

Y_UNIT_TEST(CompactionPlanner) {
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);

TLocalHelper(kikimr).CreateTestOlapTable();
Expand Down Expand Up @@ -2724,7 +2812,9 @@ Y_UNIT_TEST_SUITE(KqpOlap) {
}

Y_UNIT_TEST(MetadataMemoryManager) {
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);

TLocalHelper(kikimr).CreateTestOlapTable();
Expand Down Expand Up @@ -2781,7 +2871,9 @@ Y_UNIT_TEST_SUITE(KqpOlap) {
}

Y_UNIT_TEST(NormalizeAbsentColumn) {
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
TLocalHelper testHelper(kikimr);

Expand Down
8 changes: 6 additions & 2 deletions ydb/core/kqp/ut/olap/sparsed_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ Y_UNIT_TEST_SUITE(KqpOlapSparsed) {

class TSparsedDataTest {
private:
const TKikimrSettings Settings = TKikimrSettings().SetWithSampleTables(false);
const TKikimrSettings Settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner Kikimr;
NKikimr::NYDBTest::TControllers::TGuard<NKikimr::NYDBTest::NColumnShard::TController> CSController;
const TString StoreName;
Expand Down Expand Up @@ -304,7 +306,9 @@ Y_UNIT_TEST_SUITE(KqpOlapSparsed) {
}

Y_UNIT_TEST(AccessorActualization) {
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);

auto csController = NYDBTest::TControllers::RegisterCSControllerGuard<NYDBTest::NColumnShard::TController>();
Expand Down
9 changes: 8 additions & 1 deletion ydb/core/kqp/ut/olap/sys_view_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
auto csController = NYDBTest::TControllers::RegisterCSControllerGuard<NOlap::TWaitCompactionController>();
csController->SetOverrideBlobSplitSettings(NOlap::NSplitter::TSplitSettings());
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
Expand All @@ -137,6 +138,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
const ui32 groupsCount = 512;
{
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
Expand Down Expand Up @@ -194,6 +196,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
NKikimrConfig::TAppConfig appConfig;
auto* CSConfig = appConfig.MutableColumnShardConfig();
CSConfig->SetDefaultCompression(NKikimrSchemeOp::EColumnCodec::ColumnCodecLZ4);
CSConfig->SetAlterObjectEnabled(true);
auto settings = TKikimrSettings().SetWithSampleTables(false).SetAppConfig(appConfig);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
Expand Down Expand Up @@ -230,7 +233,9 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
ui64 rawBytes1;
ui64 bytes1;
auto csController = NYDBTest::TControllers::RegisterCSControllerGuard<NOlap::TWaitCompactionController>();
auto settings = TKikimrSettings().SetWithSampleTables(false);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
TTypedLocalHelper helper("Utf8", kikimr);
Expand Down Expand Up @@ -264,6 +269,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
NKikimrConfig::TAppConfig appConfig;
auto* CSConfig = appConfig.MutableColumnShardConfig();
CSConfig->SetDefaultCompression(NKikimrSchemeOp::EColumnCodec::ColumnCodecLZ4);
CSConfig->SetAlterObjectEnabled(true);
auto settings = TKikimrSettings().SetWithSampleTables(false).SetAppConfig(appConfig);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
Expand Down Expand Up @@ -305,6 +311,7 @@ Y_UNIT_TEST_SUITE(KqpOlapSysView) {
NKikimrConfig::TAppConfig appConfig;
auto* CSConfig = appConfig.MutableColumnShardConfig();
CSConfig->SetDefaultCompression(NKikimrSchemeOp::EColumnCodec::ColumnCodecLZ4);
CSConfig->SetAlterObjectEnabled(true);
auto settings = TKikimrSettings().SetWithSampleTables(false).SetAppConfig(appConfig);
TKikimrRunner kikimr(settings);
Tests::NCommon::TLoggerInit(kikimr).Initialize();
Expand Down
27 changes: 16 additions & 11 deletions ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5518,8 +5518,9 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
}

Y_UNIT_TEST(AlterColumnTableTtl) {
TKikimrSettings runnerSettings;
runnerSettings.WithSampleTables = false;
auto runnerSettings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TKikimrRunner kikimr(runnerSettings);
auto db = kikimr.GetTableClient();
auto session = db.CreateSession().GetValueSync().GetSession();
Expand Down Expand Up @@ -9222,9 +9223,10 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
}

Y_UNIT_TEST(AddColumnWithTtl) {
TKikimrSettings runnerSettings;
runnerSettings.WithSampleTables = false;
TTestHelper testHelper(runnerSettings);
auto settings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(settings);

TVector<TTestHelper::TColumnSchema> schema = {
TTestHelper::TColumnSchema().SetName("id").SetType(NScheme::NTypeIds::Int32).SetNullable(false),
Expand Down Expand Up @@ -9890,8 +9892,9 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
}

Y_UNIT_TEST(DropTtlColumn) {
TKikimrSettings runnerSettings;
runnerSettings.WithSampleTables = false;
auto runnerSettings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(runnerSettings);

TVector<TTestHelper::TColumnSchema> schema = {
Expand Down Expand Up @@ -11454,8 +11457,9 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
}

Y_UNIT_TEST(DropColumnAndResetTtl) {
TKikimrSettings runnerSettings;
runnerSettings.WithSampleTables = false;
auto runnerSettings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(runnerSettings);

TVector<TTestHelper::TColumnSchema> schema = {
Expand Down Expand Up @@ -11490,8 +11494,9 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
}

Y_UNIT_TEST(InitTtlSettingsOnShardStart) {
TKikimrSettings runnerSettings;
runnerSettings.WithSampleTables = false;
auto runnerSettings = TKikimrSettings()
.SetColumnShardAlterObjectEnabled(true)
.SetWithSampleTables(false);
TTestHelper testHelper(runnerSettings);

TVector<TTestHelper::TColumnSchema> schema = {
Expand Down
1 change: 1 addition & 0 deletions ydb/core/protos/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,7 @@ message TColumnShardConfig {
optional NKikimrSchemeOp.EColumnCodec DefaultCompression = 36;
optional int32 DefaultCompressionLevel = 37;
optional uint64 MemoryLimitMergeOnCompactionRawData = 38 [default = 512000000];
optional bool AlterObjectEnabled = 39 [default = false];
}

message TSchemeShardConfig {
Expand Down
1 change: 1 addition & 0 deletions ydb/core/statistics/ut_common/ut_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ TTestEnv::TTestEnv(ui32 staticNodes, ui32 dynamicNodes, bool useRealThreads)
Settings->SetUseRealThreads(useRealThreads);
Settings->AddStoragePoolType("hdd1");
Settings->AddStoragePoolType("hdd2");
Settings->SetColumnShardAlterObjectEnabled(true);

NKikimrConfig::TFeatureFlags featureFlags;
featureFlags.SetEnableStatistics(true);
Expand Down
4 changes: 4 additions & 0 deletions ydb/core/testlib/test_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ namespace Tests {
return *this;
}

TServerSettings& SetColumnShardAlterObjectEnabled(bool enable) {
AppConfig->MutableColumnShardConfig()->SetAlterObjectEnabled(enable);
return *this;
}

// Add additional grpc services
template <typename TService>
Expand Down

0 comments on commit 9cb9dec

Please sign in to comment.