Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

Commit 4a18718

Browse files
trollyxiatrollyxia
committed
misc: Address review comments
- Remove SchemalessProto/Enum SqlType APIs - Add @BetaApi - Suppress clirr failures - Change docstring etc Change-Id: Ifbeb710ac77245a741500b305c2f96374a4717e9
1 parent 7927687 commit 4a18718

9 files changed

Lines changed: 60 additions & 94 deletions

File tree

‎google-cloud-bigtable/clirr-ignored-differences.xml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,30 @@
331331
<className>com/google/cloud/bigtable/data/v2/models/sql/Statement$Builder</className>
332332
<method>*</method>
333333
</difference>
334+
<difference>
335+
<!-- BetaApi was added -->
336+
<differenceType>7012</differenceType>
337+
<className>com/google/cloud/bigtable/data/v2/models/sql/StructReader</className>
338+
<method>*getProtoMessage(*)</method>
339+
</difference>
340+
<difference>
341+
<!-- BetaApi was added -->
342+
<differenceType>7012</differenceType>
343+
<className>com/google/cloud/bigtable/data/v2/models/sql/StructReader</className>
344+
<method>*getProtoEnum(*)</method>
345+
</difference>
346+
<difference>
347+
<!-- BetaApi was added -->
348+
<differenceType>7012</differenceType>
349+
<className>com/google/cloud/bigtable/data/v2/models/sql/SqlType</className>
350+
<method>*protoOf(*)</method>
351+
</difference>
352+
<difference>
353+
<!-- BetaApi was added -->
354+
<differenceType>7012</differenceType>
355+
<className>com/google/cloud/bigtable/data/v2/models/sql/SqlType</className>
356+
<method>*enumOf(*)</method>
357+
</difference>
334358
<difference>
335359
<!-- InternalApi was updated -->
336360
<differenceType>7004</differenceType>

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/common/Type.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,10 +489,9 @@ public int hashCode() {
489489
}
490490

491491
/**
492-
* This is a special version of proto that is intended to only be used in the {@link
493-
* com.google.cloud.bigtable.data.v2.models.sql.StructReader} getters that require types. We don't
494-
* want users to need to specify the proto schema when the schema will be validated on calls to
495-
* {@link com.google.cloud.bigtable.data.v2.models.sql.StructReader} methods on the struct.
492+
* This is a special version of proto that is intended to only be used internally, to facilitate
493+
* proto schema parsing, which does not have the full information required to parse the protobuf
494+
* messages.
496495
*
497496
* <p>Any attempts to call getParserForType() will throw an exception.
498497
*/
@@ -532,10 +531,9 @@ public java.lang.String toString() {
532531
}
533532

534533
/**
535-
* This is a special version of enum that is intended to only be used in the {@link
536-
* com.google.cloud.bigtable.data.v2.models.sql.StructReader} getters that require types. We don't
537-
* want users to need to specify the enum schema when the schema will be validated on calls to
538-
* {@link com.google.cloud.bigtable.data.v2.models.sql.StructReader} methods on the struct.
534+
* This is a special version of enum that is intended to only be used internally, to facilitate
535+
* enum schema parsing, which does not have the full information required to parse the protobuf
536+
* enum messages.
539537
*
540538
* <p>Any attempts to call getForNumber() will throw an exception.
541539
*/

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/sql/SqlType.java‎

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.cloud.bigtable.data.v2.models.sql;
1717

18+
import com.google.api.core.BetaApi;
1819
import com.google.api.core.InternalApi;
1920
import com.google.cloud.Date;
2021
import com.google.cloud.bigtable.common.Type;
@@ -244,64 +245,13 @@ static <K, V> SqlType.Map<K, V> mapOf(SqlType<K> keyType, SqlType<V> valType) {
244245
return mapOf(bytes(), arrayOf(struct()));
245246
}
246247

247-
/**
248-
* Creates a fake {@link SqlType.Proto} for use on in {@link StructReader} methods that require a
249-
* {@link SqlType} to validate against. This does not specify a schema because the proto schema
250-
* will be validated on calls to the structs data accessors.
251-
*
252-
* <p>This is considered an internal implementation detail and not meant to be used by
253-
* applications.
254-
*/
255-
@InternalApi
256-
static SqlType.Proto protoOf(com.google.bigtable.v2.Type.Proto protoType) {
257-
return Type.SchemalessProto.fromProto(protoType);
258-
}
259-
260-
/**
261-
* Creates a fake {@link SqlType.Enum} for use on in {@link StructReader} methods that require a
262-
* {@link SqlType} to validate against. This does not specify a schema because the enum schema
263-
* will be validated on calls to the structs data accessors.
264-
*
265-
* <p>This is considered an internal implementation detail and not meant to be used by
266-
* applications.
267-
*/
268-
@InternalApi
269-
static SqlType.Enum enumOf(com.google.bigtable.v2.Type.Enum enumType) {
270-
return Type.SchemalessEnum.fromProto(enumType);
271-
}
272-
273-
/**
274-
* Creates a fake {@link SqlType.Proto} for use on in {@link StructReader} methods that require a
275-
* {@link SqlType} to validate against. This does not specify a schema because the proto schema
276-
* will be validated on calls to the structs data accessors.
277-
*
278-
* <p>This is considered an internal implementation detail and not meant to be used by
279-
* applications.
280-
*/
281-
@InternalApi
282-
static SqlType.Proto protoOf(String messageName) {
283-
return Type.SchemalessProto.create(messageName);
284-
}
285-
286-
/**
287-
* Creates a fake {@link SqlType.Enum} for use on in {@link StructReader} methods that require a
288-
* {@link SqlType} to validate against. This does not specify a schema because the enum schema
289-
* will be validated on calls to the structs data accessors.
290-
*
291-
* <p>This is considered an internal implementation detail and not meant to be used by
292-
* applications.
293-
*/
294-
@InternalApi
295-
static SqlType.Enum enumOf(String enumName) {
296-
return Type.SchemalessEnum.create(enumName);
297-
}
298-
299248
/**
300249
* Returns a {@link SqlType} for a protobuf message.
301250
*
302251
* @param message an instance of the message. {@code MyMessage.getDefaultInstance()} can be used.
303252
* @param <T> the message type
304253
*/
254+
@BetaApi("This feature is currently experimental and can change in the future")
305255
static <T extends AbstractMessage> SqlType.Proto<T> protoOf(T message) {
306256
return Type.Proto.create(message);
307257
}
@@ -313,6 +263,7 @@ static <T extends AbstractMessage> SqlType.Proto<T> protoOf(T message) {
313263
* MyEnum::forNumber}
314264
* @param <T> the enum type
315265
*/
266+
@BetaApi("This feature is currently experimental and can change in the future")
316267
static <T extends ProtocolMessageEnum> SqlType.Enum<T> enumOf(Function<Integer, T> method) {
317268
return Type.Enum.create(method);
318269
}
@@ -350,9 +301,9 @@ static SqlType<?> fromProto(com.google.bigtable.v2.Type proto) {
350301
com.google.bigtable.v2.Type.Map mapType = proto.getMapType();
351302
return mapOf(fromProto(mapType.getKeyType()), fromProto(mapType.getValueType()));
352303
case PROTO_TYPE:
353-
return protoOf(proto.getProtoType());
304+
return Type.SchemalessProto.fromProto(proto.getProtoType());
354305
case ENUM_TYPE:
355-
return enumOf(proto.getEnumType());
306+
return Type.SchemalessEnum.fromProto(proto.getEnumType());
356307
case KIND_NOT_SET:
357308
throw new IllegalStateException("Unrecognized Type. You may need to update your client.");
358309
default:

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/sql/StructReader.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.cloud.bigtable.data.v2.models.sql;
1717

18+
import com.google.api.core.BetaApi;
1819
import com.google.cloud.Date;
1920
import com.google.protobuf.AbstractMessage;
2021
import com.google.protobuf.ByteString;
@@ -210,6 +211,7 @@ public interface StructReader {
210211
* @see <a
211212
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#message">getDefaultInstance()</a>
212213
*/
214+
@BetaApi("This feature is currently experimental and can change in the future")
213215
<MsgType extends AbstractMessage> MsgType getProtoMessage(int columnIndex, MsgType message);
214216

215217
/**
@@ -222,6 +224,7 @@ public interface StructReader {
222224
* @see <a
223225
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#message">getDefaultInstance()</a>
224226
*/
227+
@BetaApi("This feature is currently experimental and can change in the future")
225228
<MsgType extends AbstractMessage> MsgType getProtoMessage(String columnName, MsgType message);
226229

227230
/**
@@ -234,6 +237,7 @@ public interface StructReader {
234237
* @see <a
235238
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#enum">forNumber()</a>
236239
*/
240+
@BetaApi("This feature is currently experimental and can change in the future")
237241
<EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
238242
int columnIndex, Function<Integer, EnumType> forNumber);
239243

@@ -247,6 +251,7 @@ <EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
247251
* @see <a
248252
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#enum">forNumber()</a>
249253
*/
254+
@BetaApi("This feature is currently experimental and can change in the future")
250255
<EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
251256
String columnName, Function<Integer, EnumType> forNumber);
252257
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/sql/ProtoRowsMergingStateMachine.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.bigtable.v2.PartialResultSet;
2020
import com.google.bigtable.v2.ProtoRows;
2121
import com.google.bigtable.v2.Value;
22-
import com.google.bigtable.v2.Value.KindCase;
2322
import com.google.cloud.bigtable.data.v2.internal.ProtoSqlRow;
2423
import com.google.cloud.bigtable.data.v2.internal.SqlRow;
2524
import com.google.cloud.bigtable.data.v2.models.sql.ColumnMetadata;
@@ -257,7 +256,7 @@ static void validateValueAndType(SqlType<?> type, Value value) {
257256
case PROTO:
258257
checkExpectedKind(value, Value.KindCase.BYTES_VALUE, type);
259258
case ENUM:
260-
checkExpectedKind(value, KindCase.INT_VALUE, type);
259+
checkExpectedKind(value, Value.KindCase.INT_VALUE, type);
261260
default:
262261
// This should be caught already at ResultSetMetadata creation
263262
throw new IllegalStateException("Unrecognized type: " + type);

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/AbstractProtoStructReaderTest.java‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -289,22 +289,6 @@ public void mapField_accessingProto() {
289289
SqlType.mapOf(
290290
SqlType.bytes(),
291291
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"))));
292-
assertThrows(
293-
UnsupportedOperationException.class,
294-
() ->
295-
structWithMap.getMap(
296-
"testField",
297-
SqlType.mapOf(
298-
SqlType.bytes(),
299-
SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"))));
300-
assertThrows(
301-
UnsupportedOperationException.class,
302-
() ->
303-
structWithMap.getMap(
304-
0,
305-
SqlType.mapOf(
306-
SqlType.bytes(),
307-
SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"))));
308292
assertThrows(
309293
IllegalStateException.class,
310294
() ->
@@ -372,15 +356,15 @@ public void mapField_accessingEnum() {
372356
"testField",
373357
SqlType.mapOf(
374358
SqlType.bytes(),
375-
SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"))));
359+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"))));
376360
assertThrows(
377361
UnsupportedOperationException.class,
378362
() ->
379363
structWithMap.getMap(
380364
0,
381365
SqlType.mapOf(
382366
SqlType.bytes(),
383-
SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"))));
367+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"))));
384368
assertThrows(
385369
IllegalStateException.class,
386370
() -> structWithMap.getMap("testField", SqlType.mapOf(SqlType.bytes(), SqlType.bytes())));

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/ProtoStructTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
import com.google.bigtable.v2.Type.Struct;
4848
import com.google.bigtable.v2.Value;
4949
import com.google.cloud.Date;
50+
import com.google.cloud.bigtable.common.Type.SchemalessEnum;
51+
import com.google.cloud.bigtable.common.Type.SchemalessProto;
5052
import com.google.cloud.bigtable.data.v2.models.sql.SqlType;
5153
import com.google.cloud.bigtable.data.v2.test.SingerProto.Genre;
5254
import com.google.cloud.bigtable.data.v2.test.SingerProto.Singer;
@@ -182,9 +184,9 @@ public void getColumnType_byName() {
182184
assertThat(struct.getColumnType("mapField"))
183185
.isEqualTo(SqlType.mapOf(SqlType.string(), SqlType.string()));
184186
assertThat(struct.getColumnType("protoField"))
185-
.isEqualTo(SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"));
187+
.isEqualTo(SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"));
186188
assertThat(struct.getColumnType("enumField"))
187-
.isEqualTo(SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"));
189+
.isEqualTo(SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"));
188190
}
189191

190192
@Test
@@ -203,9 +205,9 @@ public void getColumnType_byIndex() {
203205
assertThat(struct.getColumnType(10))
204206
.isEqualTo(SqlType.mapOf(SqlType.string(), SqlType.string()));
205207
assertThat(struct.getColumnType(11))
206-
.isEqualTo(SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"));
208+
.isEqualTo(SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"));
207209
assertThat(struct.getColumnType(12))
208-
.isEqualTo(SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"));
210+
.isEqualTo(SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"));
209211
}
210212

211213
@Test

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/sql/SqlTypeTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import static com.google.common.truth.Truth.assertThat;
3333

3434
import com.google.bigtable.v2.Type;
35+
import com.google.cloud.bigtable.common.Type.SchemalessEnum;
36+
import com.google.cloud.bigtable.common.Type.SchemalessProto;
3537
import com.google.cloud.bigtable.common.Type.StructWithSchema;
3638
import com.google.cloud.bigtable.data.v2.models.sql.SqlType.Code;
3739
import com.google.cloud.bigtable.data.v2.test.AlbumProto.Album;
@@ -74,8 +76,8 @@ public void setUp() {
7476
protoToJavaMapping.put(arrayType(stringType()), SqlType.arrayOf(SqlType.string()));
7577
protoToJavaMapping.put(
7678
mapType(bytesType(), stringType()), SqlType.mapOf(SqlType.bytes(), SqlType.string()));
77-
protoToJavaMapping.put(protoType("foo"), SqlType.protoOf("foo"));
78-
protoToJavaMapping.put(enumType("foo"), SqlType.enumOf("foo"));
79+
protoToJavaMapping.put(protoType("foo"), SchemalessProto.create("foo"));
80+
protoToJavaMapping.put(enumType("foo"), SchemalessEnum.create("foo"));
7981
}
8082

8183
@Test
@@ -170,8 +172,8 @@ public void typesMatch_checksMapSchema() {
170172
public void typesMatch_checksProto() {
171173
SqlType.Proto<Singer> proto = SqlType.protoOf(Singer.getDefaultInstance());
172174
SqlType.Proto<Album> anotherProto = SqlType.protoOf(Album.getDefaultInstance());
173-
SqlType.Proto schemalessProto = SqlType.protoOf("MyMessage");
174-
SqlType.Proto anotherSchemalessProto = SqlType.protoOf("MyAnotherMessage");
175+
SqlType.Proto schemalessProto = SchemalessProto.create("MyMessage");
176+
SqlType.Proto anotherSchemalessProto = SchemalessProto.create("MyAnotherMessage");
175177

176178
assertThat(SqlType.typesMatch(schemalessProto, proto)).isTrue();
177179
assertThat(SqlType.typesMatch(proto, schemalessProto)).isTrue();
@@ -183,8 +185,8 @@ public void typesMatch_checksProto() {
183185
public void typesMatch_checksEnum() {
184186
SqlType.Enum<Genre> myEnum = SqlType.enumOf(Genre::forNumber);
185187
SqlType.Enum<Format> anotherEnum = SqlType.enumOf(Format::forNumber);
186-
SqlType.Enum schemalessEnum = SqlType.enumOf("MyEnum");
187-
SqlType.Enum anotherSchemalessEnum = SqlType.enumOf("MyAnotherEnum");
188+
SqlType.Enum schemalessEnum = SchemalessEnum.create("MyEnum");
189+
SqlType.Enum anotherSchemalessEnum = SchemalessEnum.create("MyAnotherEnum");
188190

189191
assertThat(SqlType.typesMatch(schemalessEnum, myEnum)).isTrue();
190192
assertThat(SqlType.typesMatch(myEnum, schemalessEnum)).isTrue();

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/sql/ProtoRowsMergingStateMachineTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.google.bigtable.v2.ProtoRows;
4949
import com.google.bigtable.v2.ProtoRowsBatch;
5050
import com.google.bigtable.v2.Value;
51+
import com.google.cloud.bigtable.common.Type;
5152
import com.google.cloud.bigtable.data.v2.internal.ProtoResultSetMetadata;
5253
import com.google.cloud.bigtable.data.v2.internal.ProtoSqlRow;
5354
import com.google.cloud.bigtable.data.v2.models.sql.ResultSetMetadata;
@@ -659,14 +660,14 @@ public void testValidateSupportsAllTypes() {
659660
IllegalStateException.class,
660661
() ->
661662
ProtoRowsMergingStateMachine.validateValueAndType(
662-
SqlType.protoOf("test"), stringValue("test")));
663+
Type.SchemalessProto.create("test"), stringValue("test")));
663664
break;
664665
case ENUM:
665666
assertThrows(
666667
IllegalStateException.class,
667668
() ->
668669
ProtoRowsMergingStateMachine.validateValueAndType(
669-
SqlType.enumOf("test"), bytesValue("val")));
670+
Type.SchemalessEnum.create("test"), bytesValue("val")));
670671
break;
671672
default:
672673
assertWithMessage(

0 commit comments

Comments
 (0)