Skip to content

Commit 9df471f

Browse files
jepett0jepett0
committed
fixate the state before reworking YQL to public API translation in such a way that no need for public API change is needed (use AlterTableRequest to express index table change)
1 parent 33fd676 commit 9df471f

File tree

6 files changed

+36
-37
lines changed

6 files changed

+36
-37
lines changed

‎ydb/core/kqp/provider/yql_kikimr_type_ann.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1411,7 +1411,8 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over
14111411
&& name != "setTableSettings"
14121412
&& name != "addChangefeed"
14131413
&& name != "dropChangefeed"
1414-
&& name != "renameIndexTo")
1414+
&& name != "renameIndexTo"
1415+
&& name != "alterIndex")
14151416
{
14161417
ctx.AddError(TIssue(ctx.GetPosition(action.Name().Pos()),
14171418
TStringBuilder() << "Unknown alter table action: " << name));

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,6 @@ bool CheckFreezeStateAlreadySet(const TTableInfo::TPtr table, const NKikimrSchem
2828
return false;
2929
}
3030

31-
bool IsSuperUser(const NACLib::TUserToken* userToken) {
32-
if (!userToken)
33-
return false;
34-
35-
const auto& adminSids = AppData()->AdministrationAllowedSIDs;
36-
auto hasSid = [userToken](const TString& sid) -> bool {
37-
return userToken->IsExist(sid);
38-
};
39-
auto it = std::find_if(adminSids.begin(), adminSids.end(), hasSid);
40-
return (it != adminSids.end());
41-
}
42-
4331
TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table, const NKikimrSchemeOp::TTableDescription& alter,
4432
const bool shadowDataAllowed,
4533
TString& errStr, NKikimrScheme::EStatus& status, TOperationContext& context) {
@@ -639,7 +627,7 @@ ISubOperation::TPtr CreateFinalizeBuildIndexImplTable(TOperationId id, TTxState:
639627
TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context) {
640628
Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::EOperationType::ESchemeOpAlterTable);
641629

642-
auto alter = tx.GetAlterTable();
630+
const auto& alter = tx.GetAlterTable();
643631

644632
const TString& parentPathStr = tx.GetWorkingDir();
645633
const TString& name = alter.GetName();
@@ -675,14 +663,25 @@ TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const T
675663
return {CreateAlterTable(id, tx)};
676664
}
677665

678-
TVector<ISubOperation::TPtr> result;
666+
std::vector<const google::protobuf::FieldDescriptor*> fields;
667+
alter.GetReflection()->ListFields(alter, &fields);
668+
669+
TStringBuilder fieldNames;
670+
for (const auto* field : fields) {
671+
fieldNames << field->name() << ", ";
672+
}
679673

680-
// only for super user use
681-
// until correct and safe altering index api is released
682-
if (!IsSuperUser(context.UserToken.Get())) {
683-
return {CreateAlterTable(id, tx)};
674+
LOG_TRACE_S(context.Ctx, NKikimrServices::FLAT_TX_SCHEMESHARD, "Altering indexImplTable: " << alter.DebugString() << "\nfields: " << fieldNames);
675+
676+
for (const auto* field : fields) {
677+
if (field->name() != "Name"
678+
&& field->name() != "PartitionConfig") {
679+
return {CreateAlterTable(id, tx)};
680+
}
684681
}
685682

683+
TVector<ISubOperation::TPtr> result;
684+
686685
{
687686
auto tableIndexAltering = TransactionTemplate(parent.Parent().PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpAlterTableIndex);
688687
auto alterIndex = tableIndexAltering.MutableAlterTableIndex();

‎ydb/core/ydb_convert/table_description.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,6 @@ bool BuildAlterTableModifyScheme(const Ydb::Table::AlterTableRequest* req, NKiki
280280
}
281281
}
282282

283-
// TO DO: redo to if, because it the fields are not repeated, for is meaningless, it just overwrites what's been written
284283
for (const auto& alterIndex : req->alter_indexes()) {
285284
modifyScheme->SetOperationType(NKikimrSchemeOp::EOperationType::ESchemeOpAlterTable);
286285
modifyScheme->SetWorkingDir(workingDir + "/" + name + "/" + alterIndex.name());

‎ydb/core/ydb_convert/table_settings.cpp‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,21 +216,20 @@ bool FillCreateTableSettingsDesc(NKikimrSchemeOp::TTableDescription& tableDesc,
216216
}
217217

218218
bool FillPartitioningPolicy(
219-
// TO DO: rename from Getter to GetOrCreate
220-
std::function<NKikimrSchemeOp::TPartitioningPolicy*()>&& partitioningPolicyGetter,
219+
std::function<NKikimrSchemeOp::TPartitioningPolicy*()>&& partitioningPolicyGetOrCreate,
221220
const Ydb::Table::PartitioningSettings& alterSettings,
222221
Ydb::StatusIds::StatusCode& code,
223222
TString& error
224223
) {
225224
switch (alterSettings.partitioning_by_size()) {
226225
case Ydb::FeatureFlag::STATUS_UNSPECIFIED:
227226
if (alterSettings.partition_size_mb()) {
228-
partitioningPolicyGetter()->SetSizeToSplit((1 << 20) * alterSettings.partition_size_mb());
227+
partitioningPolicyGetOrCreate()->SetSizeToSplit((1 << 20) * alterSettings.partition_size_mb());
229228
}
230229
break;
231230
case Ydb::FeatureFlag::ENABLED:
232231
{
233-
auto* policy = partitioningPolicyGetter();
232+
auto* policy = partitioningPolicyGetOrCreate();
234233
if (alterSettings.partition_size_mb()) {
235234
policy->SetSizeToSplit((1 << 20) * alterSettings.partition_size_mb());
236235
} else {
@@ -250,7 +249,7 @@ bool FillPartitioningPolicy(
250249
"auto partitioning by size is disabled";
251250
return false;
252251
}
253-
partitioningPolicyGetter()->SetSizeToSplit(0);
252+
partitioningPolicyGetOrCreate()->SetSizeToSplit(0);
254253
break;
255254
}
256255
default:
@@ -263,13 +262,12 @@ bool FillPartitioningPolicy(
263262
switch (alterSettings.partitioning_by_load()) {
264263
case Ydb::FeatureFlag::STATUS_UNSPECIFIED:
265264
{
266-
// allocates the protobuf
267-
partitioningPolicyGetter();
265+
partitioningPolicyGetOrCreate();
268266
break;
269267
}
270268
case Ydb::FeatureFlag::ENABLED:
271269
{
272-
auto* policy = partitioningPolicyGetter();
270+
auto* policy = partitioningPolicyGetOrCreate();
273271
policy->MutableSplitByLoadSettings()->SetEnabled(true);
274272
if (!alterSettings.min_partitions_count()) {
275273
policy->SetMinPartitionsCount(defaultMinPartitions);
@@ -278,7 +276,7 @@ bool FillPartitioningPolicy(
278276
}
279277
case Ydb::FeatureFlag::DISABLED:
280278
{
281-
partitioningPolicyGetter()->MutableSplitByLoadSettings()->SetEnabled(false);
279+
partitioningPolicyGetOrCreate()->MutableSplitByLoadSettings()->SetEnabled(false);
282280
break;
283281
}
284282
default:
@@ -289,11 +287,11 @@ bool FillPartitioningPolicy(
289287
}
290288

291289
if (alterSettings.min_partitions_count()) {
292-
partitioningPolicyGetter()->SetMinPartitionsCount(alterSettings.min_partitions_count());
290+
partitioningPolicyGetOrCreate()->SetMinPartitionsCount(alterSettings.min_partitions_count());
293291
}
294292

295293
if (alterSettings.max_partitions_count()) {
296-
partitioningPolicyGetter()->SetMaxPartitionsCount(alterSettings.max_partitions_count());
294+
partitioningPolicyGetOrCreate()->SetMaxPartitionsCount(alterSettings.max_partitions_count());
297295
}
298296

299297
return true;
@@ -309,7 +307,6 @@ bool FillAlterTableSettingsDesc(NKikimrSchemeOp::TTableDescription& tableDesc,
309307
if (proto.has_alter_partitioning_settings()) {
310308
if (!FillPartitioningPolicy(
311309
[&partitionConfig, &changed]() {
312-
// changed at least because the partitioning policy is allocated
313310
changed = true;
314311
return partitionConfig.MutablePartitioningPolicy();
315312
},

‎ydb/library/yql/core/expr_nodes/yql_expr_nodes.json‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
{"Index": 1, "Name": "Value", "Type": "TExprBase", "Optional": true}
3131
]
3232
},
33+
{
34+
"Name": "TCoNameValueTupleList",
35+
"ListBase": "TCoNameValueTuple"
36+
},
3337
{
3438
"Name": "TCoIndex",
3539
"Base": "TExprBase",
@@ -38,17 +42,14 @@
3842
{"Index": 0, "Name": "Name", "Type": "TCoAtom"},
3943
{"Index": 1, "Name": "Type", "Type": "TCoAtom"},
4044
{"Index": 2, "Name": "Columns", "Type": "TCoAtomList"},
41-
{"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"}
45+
{"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"},
46+
{"Index": 4, "Name": "TableSettings", "Type": "TCoNameValueTupleList"}
4247
]
4348
},
4449
{
4550
"Name": "TCoIndexList",
4651
"ListBase": "TCoIndex"
4752
},
48-
{
49-
"Name": "TCoNameValueTupleList",
50-
"ListBase": "TCoNameValueTuple"
51-
},
5253
{
5354
"Name": "TCoChangefeed",
5455
"Base": "TExprBase",

‎ydb/library/yql/providers/common/provider/yql_provider.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ TWriteTableSettings ParseWriteTableSettings(TExprList node, TExprContext& ctx) {
286286
index.Columns(item.Value().Cast<TCoAtomList>());
287287
} else if (indexItemName == "dataColumns") {
288288
index.DataColumns(item.Value().Cast<TCoAtomList>());
289+
} else if (indexItemName == "tableSettings") {
290+
index.TableSettings(item.Value().Cast<TCoNameValueTupleList>());
289291
} else {
290292
YQL_ENSURE(false, "unknown index item");
291293
}

0 commit comments

Comments
 (0)