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

Commit 3fee7c3

Browse files
trollyxiatrollyxia
committed
Check unqualified message/enum names when comparing schemaless versus concrete types
Change-Id: Ide2ce0fe2e887daacc844b441276034b5d90ccfc
1 parent 65f3dbf commit 3fee7c3

3 files changed

Lines changed: 101 additions & 30 deletions

File tree

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -440,18 +440,23 @@ public static <T extends ProtocolMessageEnum> SqlType.Enum<T> create(
440440
@Override
441441
public abstract Function<Integer, T> getForNumber();
442442

443+
@Override
444+
public java.lang.String getEnumName() {
445+
T thisEnum = getForNumber().apply(0);
446+
if (thisEnum == null) {
447+
return "";
448+
}
449+
return thisEnum.getDescriptorForType().getFullName();
450+
}
451+
443452
@Override
444453
public Code getCode() {
445454
return Code.ENUM;
446455
}
447456

448457
@Override
449458
public java.lang.String toString() {
450-
T thisEnum = getForNumber().apply(0);
451-
if (thisEnum == null) {
452-
return getCode().name() + "{function=" + getForNumber() + "}";
453-
}
454-
return getCode().name() + "{enum=" + thisEnum.getDescriptorForType().getFullName() + "}";
459+
return getCode().name() + "{enum=" + getEnumName() + "}";
455460
}
456461

457462
@Override
@@ -544,7 +549,7 @@ public static SchemalessEnum create(java.lang.String enumName) {
544549
return new AutoValue_Type_SchemalessEnum(enumName);
545550
}
546551

547-
abstract java.lang.String getEnumName();
552+
public abstract java.lang.String getEnumName();
548553

549554
@Override
550555
public Function<Integer, ProtocolMessageEnum> getForNumber() {

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ interface Enum<T extends ProtocolMessageEnum> extends SqlType<T> {
166166
* @return the function to convert an integer to the enum value.
167167
*/
168168
Function<Integer, T> getForNumber();
169+
170+
String getEnumName();
169171
}
170172

171173
/** returns a {@link SqlType} for the {@code BYTES} type. */
@@ -325,6 +327,16 @@ static SqlType<?> fromProto(com.google.bigtable.v2.Type proto) {
325327
*/
326328
@InternalApi
327329
static boolean typesMatch(SqlType<?> left, SqlType<?> right) {
330+
Function<String, String> getUnqualifiedMessageName =
331+
fullMessageName -> {
332+
if (fullMessageName == null || fullMessageName.isEmpty()) {
333+
return "";
334+
}
335+
int lastDotIndex = fullMessageName.lastIndexOf('.');
336+
return (lastDotIndex == -1)
337+
? fullMessageName
338+
: fullMessageName.substring(lastDotIndex + 1);
339+
};
328340
switch (left.getCode()) {
329341
case BYTES:
330342
case STRING:
@@ -343,10 +355,13 @@ static boolean typesMatch(SqlType<?> left, SqlType<?> right) {
343355
if (left instanceof Type.SchemalessProto && right instanceof Type.SchemalessProto) {
344356
return left.equals(right);
345357
}
346-
if (left instanceof Type.SchemalessProto || right instanceof Type.SchemalessProto) {
347-
return true;
358+
if (left instanceof Type.Proto && right instanceof Type.Proto) {
359+
return left.equals(right);
348360
}
349-
return left.equals(right);
361+
// Compares mixed SchemalessProto and Proto
362+
return getUnqualifiedMessageName
363+
.apply(((SqlType.Proto) left).getMessageName())
364+
.equals(getUnqualifiedMessageName.apply(((SqlType.Proto) right).getMessageName()));
350365
}
351366
case ENUM:
352367
{
@@ -356,10 +371,13 @@ static boolean typesMatch(SqlType<?> left, SqlType<?> right) {
356371
if (left instanceof Type.SchemalessEnum && right instanceof Type.SchemalessEnum) {
357372
return left.equals(right);
358373
}
359-
if (left instanceof Type.SchemalessEnum || right instanceof Type.SchemalessEnum) {
360-
return left.getCode().equals(right.getCode());
374+
if (left instanceof Type.Enum && right instanceof Type.Enum) {
375+
return left.equals(right);
361376
}
362-
return left.equals(right);
377+
// Compares mixed SchemalessEnum and Enum
378+
return getUnqualifiedMessageName
379+
.apply(((SqlType.Enum) left).getEnumName())
380+
.equals(getUnqualifiedMessageName.apply(((SqlType.Enum) right).getEnumName()));
363381
}
364382
case STRUCT:
365383
// Don't validate fields since the field types will be validated on

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

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -170,28 +170,76 @@ public void typesMatch_checksMapSchema() {
170170

171171
@Test
172172
public void typesMatch_checksProto() {
173-
SqlType.Proto<Singer> proto = SqlType.protoOf(Singer.getDefaultInstance());
174-
SqlType.Proto<Album> anotherProto = SqlType.protoOf(Album.getDefaultInstance());
175-
SqlType.Proto schemalessProto = SchemalessProto.create("MyMessage");
176-
SqlType.Proto anotherSchemalessProto = SchemalessProto.create("MyAnotherMessage");
177-
178-
assertThat(SqlType.typesMatch(schemalessProto, proto)).isTrue();
179-
assertThat(SqlType.typesMatch(proto, schemalessProto)).isTrue();
180-
assertThat(SqlType.typesMatch(schemalessProto, anotherSchemalessProto)).isFalse();
181-
assertThat(SqlType.typesMatch(proto, anotherProto)).isFalse();
173+
SqlType.Proto<Singer> singerProto = SqlType.protoOf(Singer.getDefaultInstance());
174+
SqlType.Proto<Album> albumProto = SqlType.protoOf(Album.getDefaultInstance());
175+
SqlType.Proto schemalessSinger =
176+
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer");
177+
SqlType.Proto schemalessAlbum =
178+
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Album");
179+
180+
// Both Schemaless types
181+
assertThat(SqlType.typesMatch(schemalessSinger, schemalessAlbum)).isFalse();
182+
assertThat(
183+
SqlType.typesMatch(
184+
schemalessSinger,
185+
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer")))
186+
.isTrue();
187+
188+
// Both concrete types
189+
assertThat(SqlType.typesMatch(singerProto, albumProto)).isFalse();
190+
assertThat(SqlType.typesMatch(singerProto, SqlType.protoOf(Singer.getDefaultInstance())))
191+
.isTrue();
192+
193+
// Schemaless versus concrete types (unqualified proto message names must match)
194+
assertThat(SqlType.typesMatch(schemalessSinger, singerProto)).isTrue();
195+
assertThat(SqlType.typesMatch(singerProto, schemalessSinger)).isTrue();
196+
assertThat(SqlType.typesMatch(singerProto, SchemalessProto.create("Singer"))).isTrue();
197+
assertThat(SqlType.typesMatch(singerProto, SchemalessProto.create("foo.bar.Singer"))).isTrue();
198+
assertThat(SqlType.typesMatch(schemalessSinger, albumProto)).isFalse();
199+
assertThat(SqlType.typesMatch(albumProto, schemalessSinger)).isFalse();
200+
assertThat(SqlType.typesMatch(singerProto, SchemalessProto.create("Album"))).isFalse();
201+
assertThat(
202+
SqlType.typesMatch(
203+
singerProto,
204+
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Album")))
205+
.isFalse();
206+
assertThat(SqlType.typesMatch(singerProto, SchemalessProto.create(""))).isFalse();
182207
}
183208

184209
@Test
185210
public void typesMatch_checksEnum() {
186-
SqlType.Enum<Genre> myEnum = SqlType.enumOf(Genre::forNumber);
187-
SqlType.Enum<Format> anotherEnum = SqlType.enumOf(Format::forNumber);
188-
SqlType.Enum schemalessEnum = SchemalessEnum.create("MyEnum");
189-
SqlType.Enum anotherSchemalessEnum = SchemalessEnum.create("MyAnotherEnum");
190-
191-
assertThat(SqlType.typesMatch(schemalessEnum, myEnum)).isTrue();
192-
assertThat(SqlType.typesMatch(myEnum, schemalessEnum)).isTrue();
193-
assertThat(SqlType.typesMatch(schemalessEnum, anotherSchemalessEnum)).isFalse();
194-
assertThat(SqlType.typesMatch(myEnum, anotherEnum)).isFalse();
211+
SqlType.Enum<Genre> genreEnum = SqlType.enumOf(Genre::forNumber);
212+
SqlType.Enum<Format> formatEnum = SqlType.enumOf(Format::forNumber);
213+
SqlType.Enum schemalessGenre =
214+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre");
215+
SqlType.Enum schemalessFormat =
216+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Format");
217+
218+
// Both Schemaless types
219+
assertThat(SqlType.typesMatch(schemalessGenre, schemalessFormat)).isFalse();
220+
assertThat(
221+
SqlType.typesMatch(
222+
schemalessGenre,
223+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre")))
224+
.isTrue();
225+
226+
// Both concrete types
227+
assertThat(SqlType.typesMatch(genreEnum, formatEnum)).isFalse();
228+
assertThat(SqlType.typesMatch(genreEnum, SqlType.enumOf(Genre::forNumber))).isTrue();
229+
230+
// Schemaless versus concrete types (unqualified enum message names must match)
231+
assertThat(SqlType.typesMatch(schemalessGenre, genreEnum)).isTrue();
232+
assertThat(SqlType.typesMatch(genreEnum, schemalessGenre)).isTrue();
233+
assertThat(SqlType.typesMatch(genreEnum, SchemalessEnum.create("Genre"))).isTrue();
234+
assertThat(SqlType.typesMatch(genreEnum, SchemalessEnum.create("foo.bar.Genre"))).isTrue();
235+
assertThat(SqlType.typesMatch(schemalessGenre, formatEnum)).isFalse();
236+
assertThat(SqlType.typesMatch(formatEnum, schemalessGenre)).isFalse();
237+
assertThat(SqlType.typesMatch(genreEnum, SchemalessEnum.create("Format"))).isFalse();
238+
assertThat(
239+
SqlType.typesMatch(
240+
genreEnum, SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Format")))
241+
.isFalse();
242+
assertThat(SqlType.typesMatch(genreEnum, SchemalessEnum.create(""))).isFalse();
195243
}
196244

197245
@Test

0 commit comments

Comments
 (0)