Skip to content

Commit 0f272b0

Browse files
alex268alex268
authored
Merge pull request #388 from alex268/fix_describe_table
Added validation of entry type in describeTable method
2 parents 58b78c5 + c2e43a0 commit 0f272b0

File tree

2 files changed

+79
-31
lines changed

2 files changed

+79
-31
lines changed

‎table/src/main/java/tech/ydb/table/impl/BaseSession.java‎

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import tech.ydb.proto.ValueProtos;
3939
import tech.ydb.proto.ValueProtos.TypedValue;
4040
import tech.ydb.proto.common.CommonProtos;
41+
import tech.ydb.proto.scheme.SchemeOperationProtos;
4142
import tech.ydb.proto.table.YdbTable;
4243
import tech.ydb.table.Session;
4344
import tech.ydb.table.description.ChangefeedDescription;
@@ -661,7 +662,7 @@ public CompletableFuture<Result<TableDescription>> describeTable(String path, De
661662
String traceId = getTraceIdOrGenerateNew(settings.getTraceId());
662663
final GrpcRequestSettings grpcRequestSettings = makeGrpcRequestSettings(settings.getTimeoutDuration(), traceId);
663664
return tableRpc.describeTable(request, grpcRequestSettings)
664-
.thenApply(result -> result.map(desc -> mapDescribeTable(desc, settings)));
665+
.thenApply(res -> mapDescribeTable(res, settings));
665666
}
666667

667668
private static TableTtl mapTtlSettings(YdbTable.TtlSettings ttl) {
@@ -745,12 +746,22 @@ private static ChangefeedDescription.State mapChangefeedState(YdbTable.Changefee
745746
}
746747
}
747748

748-
private static TableDescription mapDescribeTable(
749-
YdbTable.DescribeTableResult result,
750-
DescribeTableSettings describeTableSettings
751-
) {
749+
private static Result<TableDescription> mapDescribeTable(Result<YdbTable.DescribeTableResult> describeResult,
750+
DescribeTableSettings settings) {
751+
if (!describeResult.isSuccess()) {
752+
return describeResult.map(null);
753+
}
754+
YdbTable.DescribeTableResult desc = describeResult.getValue();
755+
SchemeOperationProtos.Entry.Type entryType = desc.getSelf().getType();
756+
757+
if (entryType != SchemeOperationProtos.Entry.Type.TABLE &&
758+
entryType != SchemeOperationProtos.Entry.Type.COLUMN_TABLE) {
759+
String errorMsg = "Entry " + desc.getSelf().getName() + " with type " + entryType + " is not a table";
760+
return Result.fail(Status.of(StatusCode.SCHEME_ERROR).withIssues(Issue.of(errorMsg, Issue.Severity.ERROR)));
761+
}
762+
752763
TableDescription.Builder description = TableDescription.newBuilder();
753-
switch (result.getStoreType()) {
764+
switch (desc.getStoreType()) {
754765
case STORE_TYPE_ROW:
755766
description = description.setStoreType(TableDescription.StoreType.ROW);
756767
break;
@@ -763,13 +774,13 @@ private static TableDescription mapDescribeTable(
763774
break;
764775
}
765776

766-
for (int i = 0; i < result.getColumnsCount(); i++) {
767-
YdbTable.ColumnMeta column = result.getColumns(i);
777+
for (int i = 0; i < desc.getColumnsCount(); i++) {
778+
YdbTable.ColumnMeta column = desc.getColumns(i);
768779
description.addNonnullColumn(column.getName(), ProtoType.fromPb(column.getType()), column.getFamily());
769780
}
770-
description.setPrimaryKeys(result.getPrimaryKeyList());
771-
for (int i = 0; i < result.getIndexesCount(); i++) {
772-
YdbTable.TableIndexDescription idx = result.getIndexes(i);
781+
description.setPrimaryKeys(desc.getPrimaryKeyList());
782+
for (int i = 0; i < desc.getIndexesCount(); i++) {
783+
YdbTable.TableIndexDescription idx = desc.getIndexes(i);
773784

774785
if (idx.hasGlobalIndex()) {
775786
description.addGlobalIndex(idx.getName(), idx.getIndexColumnsList(), idx.getDataColumnsList());
@@ -783,8 +794,8 @@ private static TableDescription mapDescribeTable(
783794
description.addGlobalUniqueIndex(idx.getName(), idx.getIndexColumnsList(), idx.getDataColumnsList());
784795
}
785796
}
786-
YdbTable.TableStats tableStats = result.getTableStats();
787-
if (describeTableSettings.isIncludeTableStats() && tableStats != null) {
797+
YdbTable.TableStats tableStats = desc.getTableStats();
798+
if (settings.isIncludeTableStats() && tableStats != null) {
788799
Timestamp creationTime = tableStats.getCreationTime();
789800
Instant createdAt = creationTime == null ? null : Instant.ofEpochSecond(creationTime.getSeconds(),
790801
creationTime.getNanos());
@@ -796,26 +807,24 @@ private static TableDescription mapDescribeTable(
796807
description.setTableStats(stats);
797808

798809
List<YdbTable.PartitionStats> partitionStats = tableStats.getPartitionStatsList();
799-
if (describeTableSettings.isIncludePartitionStats() && partitionStats != null) {
810+
if (settings.isIncludePartitionStats() && partitionStats != null) {
800811
for (YdbTable.PartitionStats stat : partitionStats) {
801812
description.addPartitionStat(stat.getRowsEstimate(), stat.getStoreSize());
802813
}
803814
}
804815
}
805-
YdbTable.PartitioningSettings partitioningSettings = result.getPartitioningSettings();
806-
if (partitioningSettings != null) {
807-
PartitioningSettings settings = new PartitioningSettings();
808-
settings.setPartitionSize(partitioningSettings.getPartitionSizeMb());
809-
settings.setMinPartitionsCount(partitioningSettings.getMinPartitionsCount());
810-
settings.setMaxPartitionsCount(partitioningSettings.getMaxPartitionsCount());
811-
settings.setPartitioningByLoad(partitioningSettings.getPartitioningByLoad() == CommonProtos.
812-
FeatureFlag.Status.ENABLED);
813-
settings.setPartitioningBySize(partitioningSettings.getPartitioningBySize() == CommonProtos.
814-
FeatureFlag.Status.ENABLED);
815-
description.setPartitioningSettings(settings);
816+
YdbTable.PartitioningSettings protoPs = desc.getPartitioningSettings();
817+
if (protoPs != null) {
818+
PartitioningSettings ps = new PartitioningSettings();
819+
ps.setPartitionSize(protoPs.getPartitionSizeMb());
820+
ps.setMinPartitionsCount(protoPs.getMinPartitionsCount());
821+
ps.setMaxPartitionsCount(protoPs.getMaxPartitionsCount());
822+
ps.setPartitioningByLoad(protoPs.getPartitioningByLoad() == CommonProtos.FeatureFlag.Status.ENABLED);
823+
ps.setPartitioningBySize(protoPs.getPartitioningBySize() == CommonProtos.FeatureFlag.Status.ENABLED);
824+
description.setPartitioningSettings(ps);
816825
}
817826

818-
List<YdbTable.ColumnFamily> columnFamiliesList = result.getColumnFamiliesList();
827+
List<YdbTable.ColumnFamily> columnFamiliesList = desc.getColumnFamiliesList();
819828
if (columnFamiliesList != null) {
820829
for (YdbTable.ColumnFamily family : columnFamiliesList) {
821830
ColumnFamily.Compression compression;
@@ -831,8 +840,8 @@ private static TableDescription mapDescribeTable(
831840
);
832841
}
833842
}
834-
if (describeTableSettings.isIncludeShardKeyBounds()) {
835-
List<ValueProtos.TypedValue> shardKeyBoundsList = result.getShardKeyBoundsList();
843+
if (settings.isIncludeShardKeyBounds()) {
844+
List<ValueProtos.TypedValue> shardKeyBoundsList = desc.getShardKeyBoundsList();
836845
if (shardKeyBoundsList != null) {
837846
Optional<Value<?>> leftValue = Optional.empty();
838847
for (ValueProtos.TypedValue typedValue : shardKeyBoundsList) {
@@ -851,8 +860,8 @@ private static TableDescription mapDescribeTable(
851860
}
852861
}
853862

854-
description.setTtlSettings(mapTtlSettings(result.getTtlSettings()));
855-
for (YdbTable.ChangefeedDescription pb: result.getChangefeedsList()) {
863+
description.setTtlSettings(mapTtlSettings(desc.getTtlSettings()));
864+
for (YdbTable.ChangefeedDescription pb: desc.getChangefeedsList()) {
856865
description.addChangefeed(new ChangefeedDescription(
857866
pb.getName(),
858867
mapChangefeedMode(pb.getMode()),
@@ -863,7 +872,7 @@ private static TableDescription mapDescribeTable(
863872
));
864873
}
865874

866-
return description.build();
875+
return Result.success(description.build());
867876
}
868877

869878
private static YdbTable.TransactionSettings txSettings(Transaction.Mode transactionMode) {

‎table/src/test/java/tech/ydb/table/integration/DescribeTableTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package tech.ydb.table.integration;
2+
3+
import org.junit.Assert;
4+
import org.junit.ClassRule;
5+
import org.junit.Test;
6+
7+
import tech.ydb.core.Issue;
8+
import tech.ydb.core.Status;
9+
import tech.ydb.core.StatusCode;
10+
import tech.ydb.table.SessionRetryContext;
11+
import tech.ydb.table.impl.SimpleTableClient;
12+
import tech.ydb.table.rpc.grpc.GrpcTableRpc;
13+
import tech.ydb.test.junit4.GrpcTransportRule;
14+
15+
/**
16+
*
17+
* @author Aleksandr Gorshenin
18+
*/
19+
public class DescribeTableTest {
20+
@ClassRule
21+
public static final GrpcTransportRule YDB_TRANSPORT = new GrpcTransportRule();
22+
private static final SessionRetryContext CTX = SessionRetryContext.create(SimpleTableClient.newClient(
23+
GrpcTableRpc.useTransport(YDB_TRANSPORT)
24+
).build()).build();
25+
26+
@Test
27+
public void wrongTypeTest() {
28+
String databasePath = YDB_TRANSPORT.getDatabase();
29+
Status status = CTX.supplyResult(s -> s.describeTable(databasePath)).join().getStatus();
30+
31+
Assert.assertFalse("Unexpected success", status.isSuccess());
32+
Assert.assertEquals(StatusCode.SCHEME_ERROR, status.getCode());
33+
Issue[] issues = status.getIssues();
34+
Assert.assertEquals(1, issues.length);
35+
String entryName = databasePath.substring(1); // remove lead /
36+
Assert.assertEquals("Entry " + entryName + " with type DIRECTORY is not a table", issues[0].getMessage());
37+
Assert.assertEquals(Issue.Severity.ERROR, issues[0].getSeverity());
38+
}
39+
}

0 commit comments

Comments
 (0)