diff options
author | Daniil Demin <deminds@ydb.tech> | 2024-05-23 11:33:26 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-23 11:33:26 +0300 |
commit | f702ceca69169c3999b3d2f71aee3b99541d9e80 (patch) | |
tree | 0290abcc3feeb0ae131c186f2aab7da159644ab1 | |
parent | 6ea02c5d62a337ce152ce9e1f3bc4d430e192485 (diff) | |
download | ydb-f702ceca69169c3999b3d2f71aee3b99541d9e80.tar.gz |
ALTER INDEX to change partitioning settings of indexImplTable (#4354)
-rw-r--r-- | ydb/core/kqp/provider/yql_kikimr_exec.cpp | 203 | ||||
-rw-r--r-- | ydb/core/kqp/provider/yql_kikimr_type_ann.cpp | 3 | ||||
-rw-r--r-- | ydb/core/kqp/ut/indexes/kqp_indexes_multishard_ut.cpp | 73 | ||||
-rw-r--r-- | ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp | 110 | ||||
-rw-r--r-- | ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp | 30 | ||||
-rw-r--r-- | ydb/core/tx/schemeshard/ut_base/ut_base.cpp | 61 | ||||
-rw-r--r-- | ydb/library/yql/core/expr_nodes/yql_expr_nodes.json | 11 | ||||
-rw-r--r-- | ydb/library/yql/providers/common/provider/yql_provider.cpp | 2 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/SQLv1.g.in | 8 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/format/sql_format_ut.cpp | 6 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/node.h | 9 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/query.cpp | 343 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/sql_query.cpp | 118 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/sql_query.h | 7 | ||||
-rw-r--r-- | ydb/library/yql/sql/v1/sql_ut.cpp | 20 |
15 files changed, 608 insertions, 396 deletions
diff --git a/ydb/core/kqp/provider/yql_kikimr_exec.cpp b/ydb/core/kqp/provider/yql_kikimr_exec.cpp index efb877f370..dc89b2a57e 100644 --- a/ydb/core/kqp/provider/yql_kikimr_exec.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_exec.cpp @@ -27,6 +27,7 @@ #include <ydb/library/yql/minikql/mkql_program_builder.h> #include <ydb/core/kqp/provider/yql_kikimr_results.h> +#include <ydb/core/kqp/gateway/utils/scheme_helpers.h> namespace NYql { namespace { @@ -505,6 +506,96 @@ namespace { } } } + + bool IsPartitioningSetting(TStringBuf name) { + return name == "autoPartitioningBySize" + || name == "partitionSizeMb" + || name == "autoPartitioningByLoad" + || name == "minPartitions" + || name == "maxPartitions"; + } + + [[nodiscard]] bool ParsePartitioningSettings( + Ydb::Table::PartitioningSettings& partitioningSettings, + const TCoNameValueTuple& setting, + TExprContext& ctx + ) { + auto name = setting.Name().Value(); + if (name == "autoPartitioningBySize") { + auto val = to_lower(setting.Value().Cast<TCoAtom>().StringValue()); + if (val == "enabled") { + partitioningSettings.set_partitioning_by_size(Ydb::FeatureFlag::ENABLED); + } else if (val == "disabled") { + partitioningSettings.set_partitioning_by_size(Ydb::FeatureFlag::DISABLED); + } else { + ctx.AddError( + TIssue(ctx.GetPosition(setting.Value().Cast<TCoAtom>().Pos()), + TStringBuilder() << "Unknown feature flag '" << val << "' for auto partitioning by size" + ) + ); + return false; + } + } else if (name == "partitionSizeMb") { + ui64 value = FromString<ui64>( + setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() + ); + if (value) { + partitioningSettings.set_partition_size_mb(value); + } else { + ctx.AddError( + TIssue(ctx.GetPosition(setting.Name().Pos()), + "Can't set preferred partition size to 0. " + "To disable auto partitioning by size use 'SET AUTO_PARTITIONING_BY_SIZE DISABLED'" + ) + ); + return false; + } + } else if (name == "autoPartitioningByLoad") { + auto val = to_lower(setting.Value().Cast<TCoAtom>().StringValue()); + if (val == "enabled") { + partitioningSettings.set_partitioning_by_load(Ydb::FeatureFlag::ENABLED); + } else if (val == "disabled") { + partitioningSettings.set_partitioning_by_load(Ydb::FeatureFlag::DISABLED); + } else { + ctx.AddError( + TIssue(ctx.GetPosition(setting.Value().Cast<TCoAtom>().Pos()), + TStringBuilder() << "Unknown feature flag '" << val << "' for auto partitioning by load" + ) + ); + return false; + } + } else if (name == "minPartitions") { + ui64 value = FromString<ui64>( + setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() + ); + if (value) { + partitioningSettings.set_min_partitions_count(value); + } else { + ctx.AddError( + TIssue(ctx.GetPosition(setting.Name().Pos()), + "Can't set min partition count to 0" + ) + ); + return false; + } + } else if (name == "maxPartitions") { + ui64 value = FromString<ui64>( + setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() + ); + if (value) { + partitioningSettings.set_max_partitions_count(value); + } else { + ctx.AddError( + TIssue(ctx.GetPosition(setting.Name().Pos()), + "Can't set max partition count to 0" + ) + ); + return false; + } + } + + return true; + } } class TKiSinkPlanInfoTransformer : public TGraphTransformerBase { @@ -1290,71 +1381,10 @@ public: alterTableRequest.set_set_compaction_policy(TString( setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() )); - } else if (name == "autoPartitioningBySize") { - auto partitioningSettings = alterTableRequest.mutable_alter_partitioning_settings(); - auto val = to_lower(TString(setting.Value().Cast<TCoAtom>().Value())); - if (val == "enabled") { - partitioningSettings->set_partitioning_by_size(Ydb::FeatureFlag::ENABLED); - } else if (val == "disabled") { - partitioningSettings->set_partitioning_by_size(Ydb::FeatureFlag::DISABLED); - } else { - auto errText = TStringBuilder() << "Unknown feature flag '" - << val - << "' for auto partitioning by size"; - ctx.AddError(TIssue(ctx.GetPosition(setting.Value().Cast<TCoAtom>().Pos()), - errText)); - return SyncError(); - } - } else if (name == "partitionSizeMb") { - ui64 value = FromString<ui64>( - setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() - ); - if (value) { - auto partitioningSettings = alterTableRequest.mutable_alter_partitioning_settings(); - partitioningSettings->set_partition_size_mb(value); - } else { - ctx.AddError(TIssue(ctx.GetPosition(setting.Name().Pos()), - "Can't set preferred partition size to 0. " - "To disable auto partitioning by size use 'SET AUTO_PARTITIONING_BY_SIZE DISABLED'")); - return SyncError(); - } - } else if (name == "autoPartitioningByLoad") { - auto partitioningSettings = alterTableRequest.mutable_alter_partitioning_settings(); - TString val = to_lower(TString(setting.Value().Cast<TCoAtom>().Value())); - if (val == "enabled") { - partitioningSettings->set_partitioning_by_load(Ydb::FeatureFlag::ENABLED); - } else if (val == "disabled") { - partitioningSettings->set_partitioning_by_load(Ydb::FeatureFlag::DISABLED); - } else { - auto errText = TStringBuilder() << "Unknown feature flag '" - << val - << "' for auto partitioning by load"; - ctx.AddError(TIssue(ctx.GetPosition(setting.Value().Cast<TCoAtom>().Pos()), - errText)); - return SyncError(); - } - } else if (name == "minPartitions") { - ui64 value = FromString<ui64>( - setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() - ); - if (value) { - auto partitioningSettings = alterTableRequest.mutable_alter_partitioning_settings(); - partitioningSettings->set_min_partitions_count(value); - } else { - ctx.AddError(TIssue(ctx.GetPosition(setting.Name().Pos()), - "Can't set min partition count to 0")); - return SyncError(); - } - } else if (name == "maxPartitions") { - ui64 value = FromString<ui64>( - setting.Value().Cast<TCoDataCtor>().Literal().Cast<TCoAtom>().Value() - ); - if (value) { - auto partitioningSettings = alterTableRequest.mutable_alter_partitioning_settings(); - partitioningSettings->set_max_partitions_count(value); - } else { - ctx.AddError(TIssue(ctx.GetPosition(setting.Name().Pos()), - "Can't set max partition count to 0")); + } else if (IsPartitioningSetting(name)) { + if (!ParsePartitioningSettings( + *alterTableRequest.mutable_alter_partitioning_settings(), setting, ctx + )) { return SyncError(); } } else if (name == "keyBloomFilter") { @@ -1476,6 +1506,49 @@ public: default: YQL_ENSURE(false, "Unknown index type: " << (ui32)add_index->type_case()); } + } else if (name == "alterIndex") { + if (maybeAlter.Cast().Actions().Size() > 1) { + ctx.AddError( + TIssue(ctx.GetPosition(action.Name().Pos()), + "ALTER INDEX action cannot be combined with any other ALTER TABLE action" + ) + ); + return SyncError(); + } + auto listNode = action.Value().Cast<TCoNameValueTupleList>(); + for (const auto& indexSetting : listNode) { + auto settingName = indexSetting.Name().Value(); + if (settingName == "indexName") { + auto indexName = indexSetting.Value().Cast<TCoAtom>().StringValue(); + auto indexTablePath = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(table.Metadata->Name, indexName); + alterTableRequest.set_path(std::move(indexTablePath)); + } else if (settingName == "tableSettings") { + auto tableSettings = indexSetting.Value().Cast<TCoNameValueTupleList>(); + for (const auto& tableSetting : tableSettings) { + if (IsPartitioningSetting(tableSetting.Name().Value())) { + if (!ParsePartitioningSettings( + *alterTableRequest.mutable_alter_partitioning_settings(), tableSetting, ctx + )) { + return SyncError(); + } + } else { + ctx.AddError( + TIssue(ctx.GetPosition(tableSetting.Name().Pos()), + TStringBuilder() << "Unknown index table setting: " << name + ) + ); + return SyncError(); + } + } + } else { + ctx.AddError( + TIssue(ctx.GetPosition(indexSetting.Name().Pos()), + TStringBuilder() << "Unknown alter index setting: " << settingName + ) + ); + return SyncError(); + } + } } else if (name == "dropIndex") { auto nameNode = action.Value().Cast<TCoAtom>(); alterTableRequest.add_drop_indexes(TString(nameNode.Value())); diff --git a/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp b/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp index edc41de1a5..e7d7e494f2 100644 --- a/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp @@ -1411,7 +1411,8 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over && name != "setTableSettings" && name != "addChangefeed" && name != "dropChangefeed" - && name != "renameIndexTo") + && name != "renameIndexTo" + && name != "alterIndex") { ctx.AddError(TIssue(ctx.GetPosition(action.Name().Pos()), TStringBuilder() << "Unknown alter table action: " << name)); diff --git a/ydb/core/kqp/ut/indexes/kqp_indexes_multishard_ut.cpp b/ydb/core/kqp/ut/indexes/kqp_indexes_multishard_ut.cpp index e57642c26a..bc08cedecb 100644 --- a/ydb/core/kqp/ut/indexes/kqp_indexes_multishard_ut.cpp +++ b/ydb/core/kqp/ut/indexes/kqp_indexes_multishard_ut.cpp @@ -1605,29 +1605,7 @@ Y_UNIT_TEST_SUITE(KqpMultishardIndex) { FillTable(session); - kikimr.GetTestServer().GetRuntime()->GetAppData().AdministrationAllowedSIDs.push_back("root@builtin"); - - { // without token request is forbidded - Tests::TClient& client = kikimr.GetTestClient(); - const TString scheme = R"( - Name: "indexImplTable" - PartitionConfig { - PartitioningPolicy { - MinPartitionsCount: 1 - SizeToSplit: 100500 - FastSplitSettings { - SizeThreshold: 100500 - RowCountThreshold: 100500 - } - } - } - )"; - auto result = client.AlterTable("/Root/MultiShardIndexed/index", scheme, "user@builtin"); - UNIT_ASSERT_VALUES_EQUAL_C(result->Record.GetStatus(), NMsgBusProxy::MSTATUS_ERROR, "User must not be able to alter index impl table"); - UNIT_ASSERT_VALUES_EQUAL(result->Record.GetErrorReason(), "Administrative access denied"); - } - - { // with root token request is accepted + { // regular users should be able to alter indexImplTable's PartitionConfig Tests::TClient& client = kikimr.GetTestClient(); const TString scheme = R"( Name: "indexImplTable" @@ -1642,53 +1620,24 @@ Y_UNIT_TEST_SUITE(KqpMultishardIndex) { } } )"; - auto result = client.AlterTable("/Root/MultiShardIndexed/index", scheme, "root@builtin"); - UNIT_ASSERT_VALUES_EQUAL_C(result->Record.GetStatus(), NMsgBusProxy::MSTATUS_OK, "Super user must be able to alter partition config"); + auto result = client.AlterTable("/Root/MultiShardIndexed/index", scheme, {}); + UNIT_ASSERT_VALUES_EQUAL_C(result->Record.GetStatus(), NMsgBusProxy::MSTATUS_OK, + result->Record.ShortDebugString() + ); } - { // after alter yql works fine - const TString query(R"( + { // yql works fine after alter + const TString query = R"( SELECT * FROM `/Root/MultiShardIndexed` VIEW index ORDER BY fk DESC LIMIT 1; - )"); + )"; auto result = session.ExecuteDataQuery( - query, - TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx()) - .ExtractValueSync(); + query, + TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx() + ).ExtractValueSync(); UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); UNIT_ASSERT_VALUES_EQUAL(NYdb::FormatResultSetYson(result.GetResultSet(0)), "[[[4294967295u];[4u];[\"v4\"]]]"); } - - FillTable(session); - - { // just for sure, public api got error when alter index - auto settings = NYdb::NTable::TAlterTableSettings() - .BeginAlterPartitioningSettings() - .SetPartitionSizeMb(50) - .SetMinPartitionsCount(4) - .SetMaxPartitionsCount(5) - .EndAlterPartitioningSettings(); - - auto result = session.AlterTable("/Root/MultiShardIndexed/index/indexImplTable", settings).ExtractValueSync(); - UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString()); - } - - { // however public api is able to perform alter index if user has AlterSchema right and user is a member of the list AdministrationAllowedSIDs - auto clSettings = NYdb::NTable::TClientSettings().AuthToken("root@builtin").UseQueryCache(false); - auto client = NYdb::NTable::TTableClient(kikimr.GetDriver(), clSettings); - auto session = client.CreateSession().GetValueSync().GetSession(); - - auto settings = NYdb::NTable::TAlterTableSettings() - .BeginAlterPartitioningSettings() - .SetPartitionSizeMb(50) - .SetMinPartitionsCount(4) - .SetMaxPartitionsCount(5) - .EndAlterPartitioningSettings(); - - auto result = session.AlterTable("/Root/MultiShardIndexed/index/indexImplTable", settings).ExtractValueSync(); - UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); - } - } Y_UNIT_TEST_TWIN(DataColumnUpsertMixedSemantic, StreamLookup) { diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp index 1b25f314e4..2c321bb649 100644 --- a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp +++ b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp @@ -2382,6 +2382,116 @@ Y_UNIT_TEST_SUITE(KqpScheme) { AlterTableAddIndex(EIndexTypeSql::GlobalAsync); } + Y_UNIT_TEST(AlterTableAlterIndex) { + TKikimrRunner kikimr; + auto db = kikimr.GetTableClient(); + auto session = db.CreateSession().GetValueSync().GetSession(); + CreateSampleTablesWithIndex(session); + + constexpr int minPartitionsCount = 10; + { + auto result = session.ExecuteSchemeQuery(Sprintf(R"( + ALTER TABLE `/Root/SecondaryKeys` ALTER INDEX Index SET AUTO_PARTITIONING_MIN_PARTITIONS_COUNT %d; + )", minPartitionsCount + ) + ).ExtractValueSync(); + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + } + { + auto describe = session.DescribeTable("/Root/SecondaryKeys/Index/indexImplTable").GetValueSync(); + UNIT_ASSERT_C(describe.IsSuccess(), describe.GetIssues().ToString()); + auto indexDesc = describe.GetTableDescription(); + UNIT_ASSERT_VALUES_EQUAL(indexDesc.GetPartitioningSettings().GetMinPartitionsCount(), minPartitionsCount); + } + } + + Y_UNIT_TEST(AlterIndexImplTable) { + TKikimrRunner kikimr; + auto db = kikimr.GetTableClient(); + auto session = db.CreateSession().GetValueSync().GetSession(); + CreateSampleTablesWithIndex(session); + + constexpr int minPartitionsCount = 10; + { + auto result = session.ExecuteSchemeQuery(Sprintf(R"( + ALTER TABLE `/Root/SecondaryKeys/Index/indexImplTable` SET AUTO_PARTITIONING_MIN_PARTITIONS_COUNT %d; + )", minPartitionsCount + ) + ).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString()); + UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), + "Error: Cannot find table 'db.[/Root/SecondaryKeys/Index/indexImplTable]' because it does not exist or you do not have access permissions." + ); + } + } + + Y_UNIT_TEST(AlterIndexImplTableUsingPublicAPI) { + TKikimrRunner kikimr; + auto adminSession = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + CreateSampleTablesWithIndex(adminSession); + + auto grantPermissions = [&adminSession](const char* permissions, const char* path, const char* user) { + auto grantQuery = Sprintf(R"( + GRANT %s ON `%s` TO `%s`; + )", + permissions, path, user + ); + auto result = adminSession.ExecuteSchemeQuery(grantQuery).ExtractValueSync(); + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + }; + + // a user which does not have any implicit permissions + auto userClient = NYdb::NTable::TTableClient(kikimr.GetDriver(), NYdb::NTable::TClientSettings() + .AuthToken("user@builtin") + ); + auto userSession = userClient.CreateSession().GetValueSync().GetSession(); + + constexpr int minPartitionsCount = 10; + auto tableSettings = NYdb::NTable::TAlterTableSettings() + .BeginAlterPartitioningSettings() + .SetMinPartitionsCount(minPartitionsCount) + .EndAlterPartitioningSettings(); + + // try altering indexImplTable without ALTER SCHEMA permission + { + auto result = userSession.AlterTable("/Root/SecondaryKeys/Index/indexImplTable", tableSettings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::UNAUTHORIZED, result.GetIssues().ToString()); + UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), + "Error: Access denied for user@builtin to path Root/SecondaryKeys/Index/indexImplTable" + ); + } + // grant necessary permission + { + grantPermissions("ALTER SCHEMA", "/Root/SecondaryKeys", "user@builtin"); + auto result = userSession.AlterTable("/Root/SecondaryKeys/Index/indexImplTable", tableSettings).ExtractValueSync(); + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + } + // check result + { + grantPermissions("DESCRIBE SCHEMA", "/Root/SecondaryKeys", "user@builtin"); + auto describe = userSession.DescribeTable("/Root/SecondaryKeys/Index/indexImplTable").ExtractValueSync(); + UNIT_ASSERT_C(describe.IsSuccess(), describe.GetIssues().ToString()); + auto indexDesc = describe.GetTableDescription(); + UNIT_ASSERT_VALUES_EQUAL(indexDesc.GetPartitioningSettings().GetMinPartitionsCount(), minPartitionsCount); + } + + // try altering non-partitioning setting of indexImplTable as non-superuser + auto forbiddenSettings = NYdb::NTable::TAlterTableSettings().SetCompactionPolicy("default"); + { + auto result = userSession.AlterTable("/Root/SecondaryKeys/Index/indexImplTable", forbiddenSettings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString()); + UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), + "Error: Check failed: path: '/Root/SecondaryKeys/Index/indexImplTable', error: path is not a common path" + ); + } + // become superuser + { + kikimr.GetTestServer().GetRuntime()->GetAppData().AdministrationAllowedSIDs.emplace_back("user@builtin"); + auto result = userSession.AlterTable("/Root/SecondaryKeys/Index/indexImplTable", forbiddenSettings).ExtractValueSync(); + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + } + } + Y_UNIT_TEST(AlterTableRenameIndex) { TKikimrRunner kikimr; auto db = kikimr.GetTableClient(); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp index 0541af8836..2db8104f60 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp @@ -40,6 +40,18 @@ bool IsSuperUser(const NACLib::TUserToken* userToken) { return (it != adminSids.end()); } +template <typename TMessage> +bool CheckAllowedFields(const TMessage& message, THashSet<TString>&& allowedFields) { + std::vector<const google::protobuf::FieldDescriptor*> fields; + message.GetReflection()->ListFields(message, &fields); + for (const auto* field : fields) { + if (!allowedFields.contains(field->name())) { + return false; + } + } + return true; +} + TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table, const NKikimrSchemeOp::TTableDescription& alter, const bool shadowDataAllowed, const THashSet<TString>& localSequences, TString& errStr, NKikimrScheme::EStatus& status, TOperationContext& context) { @@ -685,7 +697,7 @@ ISubOperation::TPtr CreateFinalizeBuildIndexImplTable(TOperationId id, TTxState: TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context) { Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::EOperationType::ESchemeOpAlterTable); - auto alter = tx.GetAlterTable(); + const auto& alter = tx.GetAlterTable(); const TString& parentPathStr = tx.GetWorkingDir(); const TString& name = alter.GetName(); @@ -721,14 +733,20 @@ TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const T return {CreateAlterTable(id, tx)}; } - TVector<ISubOperation::TPtr> result; - - // only for super user use - // until correct and safe altering index api is released - if (!IsSuperUser(context.UserToken.Get())) { + // Admins can alter indexImplTable unconditionally. + // Regular users can only alter allowed fields. + if (!IsSuperUser(context.UserToken.Get()) + && (!CheckAllowedFields(alter, {"Name", "PartitionConfig"}) + || (alter.HasPartitionConfig() + && !CheckAllowedFields(alter.GetPartitionConfig(), {"PartitioningPolicy"}) + ) + ) + ) { return {CreateAlterTable(id, tx)}; } + TVector<ISubOperation::TPtr> result; + { auto tableIndexAltering = TransactionTemplate(parent.Parent().PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpAlterTableIndex); auto alterIndex = tableIndexAltering.MutableAlterTableIndex(); diff --git a/ydb/core/tx/schemeshard/ut_base/ut_base.cpp b/ydb/core/tx/schemeshard/ut_base/ut_base.cpp index 51f72985cd..f8e0f6f34f 100644 --- a/ydb/core/tx/schemeshard/ut_base/ut_base.cpp +++ b/ydb/core/tx/schemeshard/ut_base/ut_base.cpp @@ -10477,45 +10477,27 @@ Y_UNIT_TEST_SUITE(TSchemeShardTest) { NLs::NoMaxPartitionsCount }); - // request without token + // request direct alter of forbidden fields of indexImplTable TestAlterTable(runtime, ++txId, "/MyRoot/table/indexByValue/", R"( - Name: "indexImplTable" - PartitionConfig { - PartitioningPolicy { - MinPartitionsCount: 1 - SizeToSplit: 100502 - FastSplitSettings { - SizeThreshold: 100502 - RowCountThreshold: 100502 - } - } - } - )", {NKikimrScheme::StatusNameConflict}); - - { // request with not a super user token - auto request = AlterTableRequest(++txId, "/MyRoot/table/indexByValue/", R"( - Name: "indexImplTable" - PartitionConfig { - PartitioningPolicy { - MinPartitionsCount: 1 - SizeToSplit: 100501 - FastSplitSettings { - SizeThreshold: 100501 - RowCountThreshold: 100501 - } - } + Name: "indexImplTable" + KeyColumnNames: ["key", "value"] + PartitionConfig { + PartitioningPolicy { + MinPartitionsCount: 1 + SizeToSplit: 100502 + FastSplitSettings { + SizeThreshold: 100502 + RowCountThreshold: 100502 } - )"); - - auto wellCookedToken = NACLib::TUserToken(TVector<TString>{"not-a-root@builtin"}); - request->Record.SetUserToken(wellCookedToken.SerializeAsString()); - TActorId sender = runtime.AllocateEdgeActor(); - ForwardToTablet(runtime, TTestTxConfig::SchemeShard, sender, request); - TestModificationResults(runtime, txId, {TEvSchemeShard::EStatus::StatusNameConflict}); - } + } + } + )", + {TEvSchemeShard::EStatus::StatusNameConflict} + ); + env.TestWaitNotification(runtime, txId); { - auto request = AlterTableRequest(++txId, "/MyRoot/table/indexByValue/", R"( + TestAlterTable(runtime, ++txId, "/MyRoot/table/indexByValue/", R"( Name: "indexImplTable" PartitionConfig { PartitioningPolicy { @@ -10527,13 +10509,8 @@ Y_UNIT_TEST_SUITE(TSchemeShardTest) { } } } - )"); - auto wellCookedToken = NACLib::TUserToken(TVector<TString>{"true-root@builtin"}); - request->Record.SetUserToken(wellCookedToken.SerializeAsString()); - TActorId sender = runtime.AllocateEdgeActor(); - ForwardToTablet(runtime, TTestTxConfig::SchemeShard, sender, request); - TestModificationResults(runtime, txId, {TEvSchemeShard::EStatus::StatusAccepted}); - + )" + ); env.TestWaitNotification(runtime, txId); } diff --git a/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json b/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json index 1da15b8250..3aa5a6d72a 100644 --- a/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json +++ b/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json @@ -31,6 +31,10 @@ ] }, { + "Name": "TCoNameValueTupleList", + "ListBase": "TCoNameValueTuple" + }, + { "Name": "TCoIndex", "Base": "TExprBase", "Match": {"Type": "Tuple"}, @@ -38,7 +42,8 @@ {"Index": 0, "Name": "Name", "Type": "TCoAtom"}, {"Index": 1, "Name": "Type", "Type": "TCoAtom"}, {"Index": 2, "Name": "Columns", "Type": "TCoAtomList"}, - {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"} + {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"}, + {"Index": 4, "Name": "TableSettings", "Type": "TCoNameValueTupleList", "Optional": true} ] }, { @@ -46,10 +51,6 @@ "ListBase": "TCoIndex" }, { - "Name": "TCoNameValueTupleList", - "ListBase": "TCoNameValueTuple" - }, - { "Name": "TCoChangefeed", "Base": "TExprBase", "Match": {"Type": "Tuple"}, diff --git a/ydb/library/yql/providers/common/provider/yql_provider.cpp b/ydb/library/yql/providers/common/provider/yql_provider.cpp index a3d50c675d..66884d6990 100644 --- a/ydb/library/yql/providers/common/provider/yql_provider.cpp +++ b/ydb/library/yql/providers/common/provider/yql_provider.cpp @@ -286,6 +286,8 @@ TWriteTableSettings ParseWriteTableSettings(TExprList node, TExprContext& ctx) { index.Columns(item.Value().Cast<TCoAtomList>()); } else if (indexItemName == "dataColumns") { index.DataColumns(item.Value().Cast<TCoAtomList>()); + } else if (indexItemName == "tableSettings") { + index.TableSettings(item.Value().Cast<TCoNameValueTupleList>()); } else { YQL_ENSURE(false, "unknown index item"); } diff --git a/ydb/library/yql/sql/v1/SQLv1.g.in b/ydb/library/yql/sql/v1/SQLv1.g.in index 4ec002e7ff..e2edd1c723 100644 --- a/ydb/library/yql/sql/v1/SQLv1.g.in +++ b/ydb/library/yql/sql/v1/SQLv1.g.in @@ -665,6 +665,7 @@ alter_table_action: | alter_table_alter_changefeed | alter_table_drop_changefeed | alter_table_rename_index_to + | alter_table_alter_index ; alter_external_table_stmt: ALTER EXTERNAL TABLE simple_table_ref alter_external_table_action (COMMA alter_external_table_action)*; @@ -698,6 +699,7 @@ alter_table_rename_index_to: RENAME INDEX an_id TO an_id; alter_table_add_changefeed: ADD changefeed; alter_table_alter_changefeed: ALTER CHANGEFEED an_id changefeed_alter_settings; alter_table_drop_changefeed: DROP CHANGEFEED an_id; +alter_table_alter_index: ALTER INDEX an_id alter_table_alter_index_action; column_schema: an_id_schema type_name_or_bind family_relation? opt_column_constraints; family_relation: FAMILY an_id; @@ -755,6 +757,12 @@ split_boundaries: literal_value_list: LPAREN literal_value (COMMA literal_value)* RPAREN; +alter_table_alter_index_action: + alter_table_set_table_setting_uncompat + | alter_table_set_table_setting_compat + | alter_table_reset_table_setting +; + drop_table_stmt: DROP (TABLE | TABLESTORE | EXTERNAL TABLE) (IF EXISTS)? simple_table_ref; create_user_stmt: CREATE USER role_name create_user_option?; diff --git a/ydb/library/yql/sql/v1/format/sql_format_ut.cpp b/ydb/library/yql/sql/v1/format/sql_format_ut.cpp index c70af8742c..a67fff8534 100644 --- a/ydb/library/yql/sql/v1/format/sql_format_ut.cpp +++ b/ydb/library/yql/sql/v1/format/sql_format_ut.cpp @@ -454,6 +454,12 @@ Y_UNIT_TEST_SUITE(CheckSqlFormatter) { "ALTER TABLE user\n\tRESET (user, user);\n"}, {"alter table user add index user local on (user)", "ALTER TABLE user\n\tADD INDEX user LOCAL ON (user);\n"}, + {"alter table user alter index idx set setting 'foo'", + "ALTER TABLE user\n\tALTER INDEX idx SET setting 'foo';\n"}, + {"alter table user alter index idx set (setting = 'foo', another_setting = 'bar')", + "ALTER TABLE user\n\tALTER INDEX idx SET (setting = 'foo', another_setting = 'bar');\n"}, + {"alter table user alter index idx reset (setting, another_setting)", + "ALTER TABLE user\n\tALTER INDEX idx RESET (setting, another_setting);\n"}, {"alter table user drop index user", "ALTER TABLE user\n\tDROP INDEX user;\n"}, {"alter table user rename to user", diff --git a/ydb/library/yql/sql/v1/node.h b/ydb/library/yql/sql/v1/node.h index 94324e85d1..68f59e3186 100644 --- a/ydb/library/yql/sql/v1/node.h +++ b/ydb/library/yql/sql/v1/node.h @@ -1022,6 +1022,7 @@ namespace NSQLTranslationV1 { EType Type; TVector<TIdentifier> IndexColumns; TVector<TIdentifier> DataColumns; + TTableSettings TableSettings; }; struct TChangefeedSettings { @@ -1072,22 +1073,22 @@ namespace NSQLTranslationV1 { TVector<TFamilyEntry> AlterColumnFamilies; TTableSettings TableSettings; TVector<TIndexDescription> AddIndexes; + TVector<TIndexDescription> AlterIndexes; TVector<TIdentifier> DropIndexes; + TMaybe<std::pair<TIdentifier, TIdentifier>> RenameIndexTo; TMaybe<TIdentifier> RenameTo; TVector<TChangefeedDescription> AddChangefeeds; TVector<TChangefeedDescription> AlterChangefeeds; TVector<TIdentifier> DropChangefeeds; - TMaybe<std::pair<TIdentifier, TIdentifier>> RenameIndexTo; ETableType TableType = ETableType::Table; bool IsEmpty() const { return AddColumns.empty() && DropColumns.empty() && AlterColumns.empty() && AddColumnFamilies.empty() && AlterColumnFamilies.empty() && !TableSettings.IsSet() - && AddIndexes.empty() && DropIndexes.empty() + && AddIndexes.empty() && AlterIndexes.empty() && DropIndexes.empty() && !RenameIndexTo.Defined() && !RenameTo.Defined() - && AddChangefeeds.empty() && AlterChangefeeds.empty() && DropChangefeeds.empty() - && !RenameIndexTo.Defined(); + && AddChangefeeds.empty() && AlterChangefeeds.empty() && DropChangefeeds.empty(); } }; diff --git a/ydb/library/yql/sql/v1/query.cpp b/ydb/library/yql/sql/v1/query.cpp index bc63bdccc6..28762c7793 100644 --- a/ydb/library/yql/sql/v1/query.cpp +++ b/ydb/library/yql/sql/v1/query.cpp @@ -152,7 +152,125 @@ static INode::TPtr CreateIndexType(TIndexDescription::EType type, const INode& n } } -static INode::TPtr CreateIndexDesc(const TIndexDescription& index, const INode& node) { +enum class ETableSettingsParsingMode { + Create, + Alter +}; + +static INode::TPtr CreateTableSettings(const TTableSettings& tableSettings, ETableSettingsParsingMode parsingMode, const INode& node) { + // short aliases for member function calls + auto Y = [&node](auto&&... args) { return node.Y(std::forward<decltype(args)>(args)...); }; + auto Q = [&node](auto&&... args) { return node.Q(std::forward<decltype(args)>(args)...); }; + auto L = [&node](auto&&... args) { return node.L(std::forward<decltype(args)>(args)...); }; + + auto settings = Y(); + + if (tableSettings.DataSourcePath) { + settings = L(settings, Q(Y(Q("data_source_path"), tableSettings.DataSourcePath))); + } + if (tableSettings.Location) { + if (tableSettings.Location.IsSet()) { + settings = L(settings, Q(Y(Q("location"), tableSettings.Location.GetValueSet()))); + } else { + Y_ENSURE(parsingMode != ETableSettingsParsingMode::Create, "Can't reset LOCATION in create mode"); + settings = L(settings, Q(Y(Q("location")))); + } + } + for (const auto& resetableParam : tableSettings.ExternalSourceParameters) { + Y_ENSURE(resetableParam, "Empty parameter"); + if (resetableParam.IsSet()) { + const auto& [id, value] = resetableParam.GetValueSet(); + settings = L(settings, Q(Y(Q(id.Name), value))); + } else { + Y_ENSURE(parsingMode != ETableSettingsParsingMode::Create, + "Can't reset " << resetableParam.GetValueReset().Name << " in create mode" + ); + settings = L(settings, Q(Y(Q(resetableParam.GetValueReset().Name)))); + } + } + if (tableSettings.CompactionPolicy) { + settings = L(settings, Q(Y(Q("compactionPolicy"), tableSettings.CompactionPolicy))); + } + if (tableSettings.AutoPartitioningBySize) { + const auto& ref = tableSettings.AutoPartitioningBySize.GetRef(); + settings = L(settings, Q(Y(Q("autoPartitioningBySize"), BuildQuotedAtom(ref.Pos, ref.Name)))); + } + if (tableSettings.UniformPartitions && parsingMode == ETableSettingsParsingMode::Create) { + settings = L(settings, Q(Y(Q("uniformPartitions"), tableSettings.UniformPartitions))); + } + if (tableSettings.PartitionAtKeys && parsingMode == ETableSettingsParsingMode::Create) { + auto keysDesc = Y(); + for (const auto& key : tableSettings.PartitionAtKeys) { + auto columnsDesc = Y(); + for (auto column : key) { + columnsDesc = L(columnsDesc, column); + } + keysDesc = L(keysDesc, Q(columnsDesc)); + } + settings = L(settings, Q(Y(Q("partitionAtKeys"), Q(keysDesc)))); + } + if (tableSettings.PartitionSizeMb) { + settings = L(settings, Q(Y(Q("partitionSizeMb"), tableSettings.PartitionSizeMb))); + } + if (tableSettings.AutoPartitioningByLoad) { + const auto& ref = tableSettings.AutoPartitioningByLoad.GetRef(); + settings = L(settings, Q(Y(Q("autoPartitioningByLoad"), BuildQuotedAtom(ref.Pos, ref.Name)))); + } + if (tableSettings.MinPartitions) { + settings = L(settings, Q(Y(Q("minPartitions"), tableSettings.MinPartitions))); + } + if (tableSettings.MaxPartitions) { + settings = L(settings, Q(Y(Q("maxPartitions"), tableSettings.MaxPartitions))); + } + if (tableSettings.KeyBloomFilter) { + const auto& ref = tableSettings.KeyBloomFilter.GetRef(); + settings = L(settings, Q(Y(Q("keyBloomFilter"), BuildQuotedAtom(ref.Pos, ref.Name)))); + } + if (tableSettings.ReadReplicasSettings) { + settings = L(settings, Q(Y(Q("readReplicasSettings"), tableSettings.ReadReplicasSettings))); + } + if (const auto& ttl = tableSettings.TtlSettings) { + if (ttl.IsSet()) { + const auto& ttlSettings = ttl.GetValueSet(); + auto opts = Y(); + + opts = L(opts, Q(Y(Q("columnName"), BuildQuotedAtom(ttlSettings.ColumnName.Pos, ttlSettings.ColumnName.Name)))); + opts = L(opts, Q(Y(Q("expireAfter"), ttlSettings.Expr))); + + if (ttlSettings.ColumnUnit) { + opts = L(opts, Q(Y(Q("columnUnit"), Q(ToString(*ttlSettings.ColumnUnit))))); + } + + settings = L(settings, Q(Y(Q("setTtlSettings"), Q(opts)))); + } else { + YQL_ENSURE(parsingMode != ETableSettingsParsingMode::Create, "Can't reset TTL settings in create mode"); + settings = L(settings, Q(Y(Q("resetTtlSettings"), Q(Y())))); + } + } + if (const auto& tiering = tableSettings.Tiering) { + if (tiering.IsSet()) { + settings = L(settings, Q(Y(Q("setTiering"), tiering.GetValueSet()))); + } else { + YQL_ENSURE(parsingMode != ETableSettingsParsingMode::Create, "Can't reset TIERING in create mode"); + settings = L(settings, Q(Y(Q("resetTiering"), Q(Y())))); + } + } + if (tableSettings.StoreExternalBlobs) { + const auto& ref = tableSettings.StoreExternalBlobs.GetRef(); + settings = L(settings, Q(Y(Q("storeExternalBlobs"), BuildQuotedAtom(ref.Pos, ref.Name)))); + } + if (tableSettings.StoreType && parsingMode == ETableSettingsParsingMode::Create) { + const auto& ref = tableSettings.StoreType.GetRef(); + settings = L(settings, Q(Y(Q("storeType"), BuildQuotedAtom(ref.Pos, ref.Name)))); + } + if (tableSettings.PartitionByHashFunction && parsingMode == ETableSettingsParsingMode::Create) { + settings = L(settings, Q(Y(Q("partitionByHashFunction"), tableSettings.PartitionByHashFunction))); + } + + return settings; +} + +static INode::TPtr CreateIndexDesc(const TIndexDescription& index, ETableSettingsParsingMode parsingMode, const INode& node) { auto indexColumns = node.Y(); for (const auto& col : index.IndexColumns) { indexColumns = node.L(indexColumns, BuildQuotedAtom(col.Pos, col.Name)); @@ -163,10 +281,32 @@ static INode::TPtr CreateIndexDesc(const TIndexDescription& index, const INode& } const auto& indexType = node.Y(node.Q("indexType"), CreateIndexType(index.Type, node)); const auto& indexName = node.Y(node.Q("indexName"), BuildQuotedAtom(index.Name.Pos, index.Name.Name)); - return node.Y(node.Q(indexName), - node.Q(indexType), - node.Q(node.Y(node.Q("indexColumns"), node.Q(indexColumns))), - node.Q(node.Y(node.Q("dataColumns"), node.Q(dataColumns)))); + auto indexNode = node.Y( + node.Q(indexName), + node.Q(indexType), + node.Q(node.Y(node.Q("indexColumns"), node.Q(indexColumns))), + node.Q(node.Y(node.Q("dataColumns"), node.Q(dataColumns))) + ); + if (index.TableSettings.IsSet()) { + const auto& tableSettings = node.Y( + node.Q("tableSettings"), + node.Q(CreateTableSettings(index.TableSettings, parsingMode, node)) + ); + indexNode = node.L(indexNode, tableSettings); + } + return indexNode; +} + +static INode::TPtr CreateAlterIndex(const TIndexDescription& index, const INode& node) { + const auto& indexName = node.Y(node.Q("indexName"), BuildQuotedAtom(index.Name.Pos, index.Name.Name)); + const auto& tableSettings = node.Y( + node.Q("tableSettings"), + node.Q(CreateTableSettings(index.TableSettings, ETableSettingsParsingMode::Alter, node)) + ); + return node.Y( + node.Q(indexName), + node.Q(tableSettings) + ); } static INode::TPtr CreateChangefeedDesc(const TChangefeedDescription& desc, const INode& node) { @@ -1033,7 +1173,7 @@ public: } for (const auto& index : Params.Indexes) { - const auto& desc = CreateIndexDesc(index, *this); + const auto& desc = CreateIndexDesc(index, ETableSettingsParsingMode::Create, *this); opts = L(opts, Q(Y(Q("index"), Q(desc)))); } @@ -1043,96 +1183,9 @@ public: } if (Params.TableSettings.IsSet()) { - auto settings = Y(); - - if (Params.TableSettings.DataSourcePath) { - settings = L(settings, Q(Y(Q("data_source_path"), Params.TableSettings.DataSourcePath))); - } - if (Params.TableSettings.Location) { - Y_ENSURE(Params.TableSettings.Location.IsSet(), "Can't reset LOCATION in create mode"); - settings = L(settings, Q(Y(Q("location"), Params.TableSettings.Location.GetValueSet()))); - } - for (const auto& resetableParam: Params.TableSettings.ExternalSourceParameters) { - Y_ENSURE(resetableParam, "Empty parameter"); - Y_ENSURE(resetableParam.IsSet(), "Can't reset " << resetableParam.GetValueReset().Name << " in create mode"); - const auto& [id, value] = resetableParam.GetValueSet(); - settings = L(settings, Q(Y(Q(id.Name), value))); - } - if (Params.TableSettings.CompactionPolicy) { - settings = L(settings, Q(Y(Q("compactionPolicy"), Params.TableSettings.CompactionPolicy))); - } - if (Params.TableSettings.AutoPartitioningBySize) { - const auto& ref = Params.TableSettings.AutoPartitioningBySize.GetRef(); - settings = L(settings, Q(Y(Q("autoPartitioningBySize"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.UniformPartitions) { - settings = L(settings, Q(Y(Q("uniformPartitions"), Params.TableSettings.UniformPartitions))); - } - if (Params.TableSettings.PartitionAtKeys) { - auto keysDesc = Y(); - for (const auto& key : Params.TableSettings.PartitionAtKeys) { - auto columnsDesc = Y(); - for (auto column : key) { - columnsDesc = L(columnsDesc, column); - } - keysDesc = L(keysDesc, Q(columnsDesc)); - } - settings = L(settings, Q(Y(Q("partitionAtKeys"), Q(keysDesc)))); - } - if (Params.TableSettings.PartitionSizeMb) { - settings = L(settings, Q(Y(Q("partitionSizeMb"), Params.TableSettings.PartitionSizeMb))); - } - if (Params.TableSettings.AutoPartitioningByLoad) { - const auto& ref = Params.TableSettings.AutoPartitioningByLoad.GetRef(); - settings = L(settings, Q(Y(Q("autoPartitioningByLoad"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.MinPartitions) { - settings = L(settings, Q(Y(Q("minPartitions"), Params.TableSettings.MinPartitions))); - } - if (Params.TableSettings.MaxPartitions) { - settings = L(settings, Q(Y(Q("maxPartitions"), Params.TableSettings.MaxPartitions))); - } - if (Params.TableSettings.KeyBloomFilter) { - const auto& ref = Params.TableSettings.KeyBloomFilter.GetRef(); - settings = L(settings, Q(Y(Q("keyBloomFilter"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.ReadReplicasSettings) { - settings = L(settings, Q(Y(Q("readReplicasSettings"), Params.TableSettings.ReadReplicasSettings))); - } - if (const auto& ttl = Params.TableSettings.TtlSettings) { - if (ttl.IsSet()) { - const auto& ttlSettings = ttl.GetValueSet(); - auto opts = Y(); - - opts = L(opts, Q(Y(Q("columnName"), BuildQuotedAtom(ttlSettings.ColumnName.Pos, ttlSettings.ColumnName.Name)))); - opts = L(opts, Q(Y(Q("expireAfter"), ttlSettings.Expr))); - - if (ttlSettings.ColumnUnit) { - opts = L(opts, Q(Y(Q("columnUnit"), Q(ToString(*ttlSettings.ColumnUnit))))); - } - - settings = L(settings, Q(Y(Q("setTtlSettings"), Q(opts)))); - } else { - YQL_ENSURE(false, "Can't reset TTL settings"); - } - } - if (Params.TableSettings.Tiering) { - YQL_ENSURE(Params.TableSettings.Tiering.IsSet(), "Can't reset TIERING in create mode"); - settings = L(settings, Q(Y(Q("setTiering"), Params.TableSettings.Tiering.GetValueSet()))); - } - if (Params.TableSettings.StoreExternalBlobs) { - const auto& ref = Params.TableSettings.StoreExternalBlobs.GetRef(); - settings = L(settings, Q(Y(Q("storeExternalBlobs"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.StoreType) { - const auto& ref = Params.TableSettings.StoreType.GetRef(); - settings = L(settings, Q(Y(Q("storeType"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.PartitionByHashFunction) { - settings = L(settings, Q(Y(Q("partitionByHashFunction"), Params.TableSettings.PartitionByHashFunction))); - } - - opts = L(opts, Q(Y(Q("tableSettings"), Q(settings)))); + opts = L(opts, Q(Y(Q("tableSettings"), Q( + CreateTableSettings(Params.TableSettings, ETableSettingsParsingMode::Create, *this) + )))); } switch (Params.TableType) { @@ -1321,100 +1374,26 @@ public: } if (Params.TableSettings.IsSet()) { - auto settings = Y(); - if (Params.TableSettings.DataSourcePath) { - settings = L(settings, Q(Y(Q("data_source_path"), Params.TableSettings.DataSourcePath))); - } - if (Params.TableSettings.Location) { - if (Params.TableSettings.Location.IsSet()) { - settings = L(settings, Q(Y(Q("location"), Params.TableSettings.Location.GetValueSet()))); - } else { - settings = L(settings, Q(Y(Q("location")))); - } - } - for (const auto& resetableParam: Params.TableSettings.ExternalSourceParameters) { - Y_ENSURE(resetableParam, "Empty parameter"); - if (resetableParam.IsSet()) { - const auto& [id, value] = resetableParam.GetValueSet(); - settings = L(settings, Q(Y(Q(id.Name), value))); - } else { - settings = L(settings, Q(Y(Q(resetableParam.GetValueReset().Name)))); - } - } - if (Params.TableSettings.CompactionPolicy) { - settings = L(settings, Q(Y(Q("compactionPolicy"), Params.TableSettings.CompactionPolicy))); - } - if (Params.TableSettings.AutoPartitioningBySize) { - const auto& ref = Params.TableSettings.AutoPartitioningBySize.GetRef(); - settings = L(settings, Q(Y(Q("autoPartitioningBySize"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.PartitionSizeMb) { - settings = L(settings, Q(Y(Q("partitionSizeMb"), Params.TableSettings.PartitionSizeMb))); - } - if (Params.TableSettings.AutoPartitioningByLoad) { - const auto& ref = Params.TableSettings.AutoPartitioningByLoad.GetRef(); - settings = L(settings, Q(Y(Q("autoPartitioningByLoad"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.MinPartitions) { - settings = L(settings, Q(Y(Q("minPartitions"), Params.TableSettings.MinPartitions))); - } - if (Params.TableSettings.MaxPartitions) { - settings = L(settings, Q(Y(Q("maxPartitions"), Params.TableSettings.MaxPartitions))); - } - if (Params.TableSettings.KeyBloomFilter) { - const auto& ref = Params.TableSettings.KeyBloomFilter.GetRef(); - settings = L(settings, Q(Y(Q("keyBloomFilter"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - if (Params.TableSettings.ReadReplicasSettings) { - settings = L(settings, Q(Y(Q("readReplicasSettings"), Params.TableSettings.ReadReplicasSettings))); - } - if (const auto& ttl = Params.TableSettings.TtlSettings) { - if (ttl.IsSet()) { - const auto& ttlSettings = ttl.GetValueSet(); - auto opts = Y(); - - opts = L(opts, Q(Y(Q("columnName"), BuildQuotedAtom(ttlSettings.ColumnName.Pos, ttlSettings.ColumnName.Name)))); - opts = L(opts, Q(Y(Q("expireAfter"), ttlSettings.Expr))); - - if (ttlSettings.ColumnUnit) { - opts = L(opts, Q(Y(Q("columnUnit"), Q(ToString(*ttlSettings.ColumnUnit))))); - } - - settings = L(settings, Q(Y(Q("setTtlSettings"), Q(opts)))); - } else { - settings = L(settings, Q(Y(Q("resetTtlSettings"), Q(Y())))); - } - } - if (const auto& tiering = Params.TableSettings.Tiering) { - if (tiering.IsSet()) { - settings = L(settings, Q(Y(Q("setTiering"), tiering.GetValueSet()))); - } else { - settings = L(settings, Q(Y(Q("resetTiering"), Q(Y())))); - } - } - if (Params.TableSettings.StoreExternalBlobs) { - const auto& ref = Params.TableSettings.StoreExternalBlobs.GetRef(); - settings = L(settings, Q(Y(Q("storeExternalBlobs"), BuildQuotedAtom(ref.Pos, ref.Name)))); - } - actions = L(actions, Q(Y(Q("setTableSettings"), Q(settings)))); + actions = L(actions, Q(Y(Q("setTableSettings"), Q( + CreateTableSettings(Params.TableSettings, ETableSettingsParsingMode::Alter, *this) + )))); } for (const auto& index : Params.AddIndexes) { - const auto& desc = CreateIndexDesc(index, *this); + const auto& desc = CreateIndexDesc(index, ETableSettingsParsingMode::Alter, *this); actions = L(actions, Q(Y(Q("addIndex"), Q(desc)))); } + for (const auto& index : Params.AlterIndexes) { + const auto& desc = CreateAlterIndex(index, *this); + actions = L(actions, Q(Y(Q("alterIndex"), Q(desc)))); + } + for (const auto& id : Params.DropIndexes) { auto indexName = BuildQuotedAtom(id.Pos, id.Name); actions = L(actions, Q(Y(Q("dropIndex"), indexName))); } - if (Params.RenameTo) { - auto destination = ctx.GetPrefixedPath(Scoped->CurrService, Scoped->CurrCluster, - TDeferredAtom(Params.RenameTo->Pos, Params.RenameTo->Name)); - actions = L(actions, Q(Y(Q("renameTo"), destination))); - } - if (Params.RenameIndexTo) { auto src = BuildQuotedAtom(Params.RenameIndexTo->first.Pos, Params.RenameIndexTo->first.Name); auto dst = BuildQuotedAtom(Params.RenameIndexTo->second.Pos, Params.RenameIndexTo->second.Name); @@ -1427,6 +1406,12 @@ public: actions = L(actions, Q(Y(Q("renameIndexTo"), Q(desc)))); } + if (Params.RenameTo) { + auto destination = ctx.GetPrefixedPath(Scoped->CurrService, Scoped->CurrCluster, + TDeferredAtom(Params.RenameTo->Pos, Params.RenameTo->Name)); + actions = L(actions, Q(Y(Q("renameTo"), destination))); + } + for (const auto& cf : Params.AddChangefeeds) { const auto& desc = CreateChangefeedDesc(cf, *this); actions = L(actions, Q(Y(Q("addChangefeed"), Q(desc)))); diff --git a/ydb/library/yql/sql/v1/sql_query.cpp b/ydb/library/yql/sql/v1/sql_query.cpp index 4cdc20727f..db408abdd4 100644 --- a/ydb/library/yql/sql/v1/sql_query.cpp +++ b/ydb/library/yql/sql/v1/sql_query.cpp @@ -1379,7 +1379,7 @@ bool TSqlQuery::AlterTableAction(const TRule_alter_table_action& node, TAlterTab case TRule_alter_table_action::kAltAlterTableAction6: { // SET (uncompat) const auto& setRule = node.GetAlt_alter_table_action6().GetRule_alter_table_set_table_setting_uncompat1(); - if (!AlterTableSetTableSetting(setRule, params)) { + if (!AlterTableSetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1387,7 +1387,7 @@ bool TSqlQuery::AlterTableAction(const TRule_alter_table_action& node, TAlterTab case TRule_alter_table_action::kAltAlterTableAction7: { // SET (compat) const auto& setRule = node.GetAlt_alter_table_action7().GetRule_alter_table_set_table_setting_compat1(); - if (!AlterTableSetTableSetting(setRule, params)) { + if (!AlterTableSetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1395,7 +1395,7 @@ bool TSqlQuery::AlterTableAction(const TRule_alter_table_action& node, TAlterTab case TRule_alter_table_action::kAltAlterTableAction8: { // RESET const auto& setRule = node.GetAlt_alter_table_action8().GetRule_alter_table_reset_table_setting1(); - if (!AlterTableResetTableSetting(setRule, params)) { + if (!AlterTableResetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1460,6 +1460,14 @@ bool TSqlQuery::AlterTableAction(const TRule_alter_table_action& node, TAlterTab AlterTableRenameIndexTo(renameTo, params); break; } + case TRule_alter_table_action::kAltAlterTableAction16: { + // ALTER INDEX + const auto& rule = node.GetAlt_alter_table_action16().GetRule_alter_table_alter_index1(); + if (!AlterTableAlterIndex(rule, params)) { + return false; + } + break; + } case TRule_alter_table_action::ALT_NOT_SET: AltNotImplemented("alter_table_action", node); @@ -1495,7 +1503,7 @@ bool TSqlQuery::AlterExternalTableAction(const TRule_alter_external_table_action case TRule_alter_external_table_action::kAltAlterExternalTableAction3: { // SET (uncompat) const auto& setRule = node.GetAlt_alter_external_table_action3().GetRule_alter_table_set_table_setting_uncompat1(); - if (!AlterTableSetTableSetting(setRule, params)) { + if (!AlterTableSetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1503,7 +1511,7 @@ bool TSqlQuery::AlterExternalTableAction(const TRule_alter_external_table_action case TRule_alter_external_table_action::kAltAlterExternalTableAction4: { // SET (compat) const auto& setRule = node.GetAlt_alter_external_table_action4().GetRule_alter_table_set_table_setting_compat1(); - if (!AlterTableSetTableSetting(setRule, params)) { + if (!AlterTableSetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1511,7 +1519,7 @@ bool TSqlQuery::AlterExternalTableAction(const TRule_alter_external_table_action case TRule_alter_external_table_action::kAltAlterExternalTableAction5: { // RESET const auto& setRule = node.GetAlt_alter_external_table_action5().GetRule_alter_table_reset_table_setting1(); - if (!AlterTableResetTableSetting(setRule, params)) { + if (!AlterTableResetTableSetting(setRule, params.TableSettings, params.TableType)) { return false; } break; @@ -1603,44 +1611,58 @@ bool TSqlQuery::AlterTableAlterFamily(const TRule_alter_table_alter_column_famil return true; } -bool TSqlQuery::AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_uncompat& node, - TAlterTableParameters& params) -{ - if (!StoreTableSettingsEntry(IdEx(node.GetRule_an_id2(), *this), node.GetRule_table_setting_value3(), - params.TableSettings, params.TableType, true)) { - return false; - } - return true; +bool TSqlQuery::AlterTableSetTableSetting( + const TRule_alter_table_set_table_setting_uncompat& node, TTableSettings& tableSettings, ETableType tableType +) { + return StoreTableSettingsEntry( + IdEx(node.GetRule_an_id2(), *this), + node.GetRule_table_setting_value3(), + tableSettings, + tableType, + true + ); } -bool TSqlQuery::AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_compat& node, - TAlterTableParameters& params) -{ +bool TSqlQuery::AlterTableSetTableSetting( + const TRule_alter_table_set_table_setting_compat& node, TTableSettings& tableSettings, ETableType tableType +) { + const auto storeSetting = [&](const TRule_alter_table_setting_entry& entry) { + return StoreTableSettingsEntry( + IdEx(entry.GetRule_an_id1(), *this), + entry.GetRule_table_setting_value3(), + tableSettings, + tableType, + true + ); + }; + const auto& firstEntry = node.GetRule_alter_table_setting_entry3(); - if (!StoreTableSettingsEntry(IdEx(firstEntry.GetRule_an_id1(), *this), firstEntry.GetRule_table_setting_value3(), - params.TableSettings, params.TableType, true)) { + if (!storeSetting(firstEntry)) { return false; } - for (auto& block : node.GetBlock4()) { + for (const auto& block : node.GetBlock4()) { const auto& entry = block.GetRule_alter_table_setting_entry2(); - if (!StoreTableSettingsEntry(IdEx(entry.GetRule_an_id1(), *this), entry.GetRule_table_setting_value3(), - params.TableSettings, params.TableType, true)) { + if (!storeSetting(entry)) { return false; } } return true; } -bool TSqlQuery::AlterTableResetTableSetting(const TRule_alter_table_reset_table_setting& node, - TAlterTableParameters& params) -{ +bool TSqlQuery::AlterTableResetTableSetting( + const TRule_alter_table_reset_table_setting& node, TTableSettings& tableSettings, ETableType tableType +) { + const auto resetSetting = [&](const TRule_an_id& id) { + return ResetTableSettingsEntry(IdEx(id, *this), tableSettings, tableType); + }; + const auto& firstEntry = node.GetRule_an_id3(); - if (!ResetTableSettingsEntry(IdEx(firstEntry, *this), params.TableSettings, params.TableType)) { + if (!resetSetting(firstEntry)) { return false; } - for (auto& block : node.GetBlock4()) { + for (const auto& block : node.GetBlock4()) { const auto& entry = block.GetRule_an_id2(); - if (!ResetTableSettingsEntry(IdEx(entry, *this), params.TableSettings, params.TableType)) { + if (!resetSetting(entry)) { return false; } } @@ -1669,6 +1691,46 @@ void TSqlQuery::AlterTableRenameIndexTo(const TRule_alter_table_rename_index_to& params.RenameIndexTo = std::make_pair(src, dst); } +bool TSqlQuery::AlterTableAlterIndex(const TRule_alter_table_alter_index& node, TAlterTableParameters& params) { + const auto indexName = IdEx(node.GetRule_an_id3(), *this); + params.AlterIndexes.emplace_back(indexName); + TTableSettings& indexTableSettings = params.AlterIndexes.back().TableSettings; + + const auto& action = node.GetRule_alter_table_alter_index_action4(); + + switch (action.Alt_case()) { + case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction1: { + // SET setting value + const auto& rule = action.GetAlt_alter_table_alter_index_action1().GetRule_alter_table_set_table_setting_uncompat1(); + if (!AlterTableSetTableSetting(rule, indexTableSettings, params.TableType)) { + return false; + } + break; + } + case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction2: { + // SET (setting1 = value1, ...) + const auto& rule = action.GetAlt_alter_table_alter_index_action2().GetRule_alter_table_set_table_setting_compat1(); + if (!AlterTableSetTableSetting(rule, indexTableSettings, params.TableType)) { + return false; + } + break; + } + case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction3: { + // RESET (setting1, ...) + const auto& rule = action.GetAlt_alter_table_alter_index_action3().GetRule_alter_table_reset_table_setting1(); + if (!AlterTableResetTableSetting(rule, indexTableSettings, params.TableType)) { + return false; + } + break; + } + case TRule_alter_table_alter_index_action::ALT_NOT_SET: + AltNotImplemented("alter_table_alter_index_action", action); + return false; + } + + return true; +} + bool TSqlQuery::AlterTableAddChangefeed(const TRule_alter_table_add_changefeed& node, TAlterTableParameters& params) { TSqlExpression expr(Ctx, Mode); return CreateChangefeed(node.GetRule_changefeed2(), expr, params.AddChangefeeds); diff --git a/ydb/library/yql/sql/v1/sql_query.h b/ydb/library/yql/sql/v1/sql_query.h index 97301f96ab..32dd9605cd 100644 --- a/ydb/library/yql/sql/v1/sql_query.h +++ b/ydb/library/yql/sql/v1/sql_query.h @@ -31,9 +31,9 @@ private: bool AlterTableAlterColumn(const TRule_alter_table_alter_column& node, TAlterTableParameters& params); bool AlterTableAddFamily(const TRule_family_entry& node, TAlterTableParameters& params); bool AlterTableAlterFamily(const TRule_alter_table_alter_column_family& node, TAlterTableParameters& params); - bool AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_uncompat& node, TAlterTableParameters& params); - bool AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_compat& node, TAlterTableParameters& params); - bool AlterTableResetTableSetting(const TRule_alter_table_reset_table_setting& node, TAlterTableParameters& params); + bool AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_uncompat& node, TTableSettings& tableSettings, ETableType tableType); + bool AlterTableSetTableSetting(const TRule_alter_table_set_table_setting_compat& node, TTableSettings& tableSettings, ETableType tableType); + bool AlterTableResetTableSetting(const TRule_alter_table_reset_table_setting& node, TTableSettings& tableSettings, ETableType tableType); bool AlterTableAddIndex(const TRule_alter_table_add_index& node, TAlterTableParameters& params); void AlterTableDropIndex(const TRule_alter_table_drop_index& node, TAlterTableParameters& params); void AlterTableRenameTo(const TRule_alter_table_rename_to& node, TAlterTableParameters& params); @@ -41,6 +41,7 @@ private: bool AlterTableAlterChangefeed(const TRule_alter_table_alter_changefeed& node, TAlterTableParameters& params); void AlterTableDropChangefeed(const TRule_alter_table_drop_changefeed& node, TAlterTableParameters& params); void AlterTableRenameIndexTo(const TRule_alter_table_rename_index_to& node, TAlterTableParameters& params); + bool AlterTableAlterIndex(const TRule_alter_table_alter_index& node, TAlterTableParameters& params); TNodePtr PragmaStatement(const TRule_pragma_stmt& stmt, bool& success); void AddStatementToBlocks(TVector<TNodePtr>& blocks, TNodePtr node); bool ParseTableStoreFeatures(std::map<TString, TDeferredAtom> & result, const TRule_alter_table_store_action & actions); diff --git a/ydb/library/yql/sql/v1/sql_ut.cpp b/ydb/library/yql/sql/v1/sql_ut.cpp index 7089bc02e6..2ef0ef36c3 100644 --- a/ydb/library/yql/sql/v1/sql_ut.cpp +++ b/ydb/library/yql/sql/v1/sql_ut.cpp @@ -2494,7 +2494,25 @@ Y_UNIT_TEST_SUITE(SqlParsingOnly) { Y_UNIT_TEST(AlterTableAddIndexWithIsNotSupported) { ExpectFailWithError("USE plato; ALTER TABLE table ADD INDEX idx LOCAL WITH (a=b, c=d, e=f) ON (col)", - "<main>:1:40: Error: local: alternative is not implemented yet: 720:7: local_index\n"); + "<main>:1:40: Error: local: alternative is not implemented yet: 722:7: local_index\n"); + } + + Y_UNIT_TEST(AlterTableAlterIndexSetPartitioningIsCorrect) { + const auto result = SqlToYql("USE plato; ALTER TABLE table ALTER INDEX index SET AUTO_PARTITIONING_MIN_PARTITIONS_COUNT 10"); + UNIT_ASSERT_C(result.IsOk(), result.Issues.ToString()); + } + + Y_UNIT_TEST(AlterTableAlterIndexSetMultiplePartitioningSettings) { + const auto result = SqlToYql("USE plato; ALTER TABLE table ALTER INDEX index SET " + "(AUTO_PARTITIONING_BY_LOAD = ENABLED, AUTO_PARTITIONING_MIN_PARTITIONS_COUNT = 10)" + ); + UNIT_ASSERT_C(result.IsOk(), result.Issues.ToString()); + } + + Y_UNIT_TEST(AlterTableAlterIndexResetPartitioningIsNotSupported) { + ExpectFailWithError("USE plato; ALTER TABLE table ALTER INDEX index RESET (AUTO_PARTITIONING_MIN_PARTITIONS_COUNT)", + "<main>:1:55: Error: AUTO_PARTITIONING_MIN_PARTITIONS_COUNT reset is not supported\n" + ); } Y_UNIT_TEST(OptionalAliases) { |