Skip to content

Commit c1ea3c5

Browse files
nvamelichevnvamelichev
committed
#149: Support NOT NULL columns via @Column(notNull=true) 
1 parent 544e77f commit c1ea3c5

File tree

17 files changed

+277
-72
lines changed

17 files changed

+277
-72
lines changed

‎databind/src/main/java/tech/ydb/yoj/databind/converter/NotNullColumn.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package tech.ydb.yoj.databind.converter;
2+
3+
import tech.ydb.yoj.databind.schema.Column;
4+
5+
import java.lang.annotation.Retention;
6+
import java.lang.annotation.Target;
7+
8+
import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
9+
import static java.lang.annotation.ElementType.FIELD;
10+
import static java.lang.annotation.ElementType.RECORD_COMPONENT;
11+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
12+
13+
/**
14+
* Signifies that the column stored in the database does not accept {@code NULL} values.
15+
*
16+
* @see Column#notNull
17+
*/
18+
@Column(notNull = true)
19+
@Target({FIELD, RECORD_COMPONENT, ANNOTATION_TYPE})
20+
@Retention(RUNTIME)
21+
public @interface NotNullColumn {
22+
}

‎databind/src/main/java/tech/ydb/yoj/databind/schema/Column.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,18 @@
5858
*/
5959
String dbTypeQualifier() default "";
6060

61+
/**
62+
* Specifies whether only non-{@code NULL} values can be stored in this column. Defaults to {@code false} (allow {@code NULL} values).<br>
63+
* Note that this is orthogonal to Java nullness annotations, because YOJ uses {@code null} values for ID fields as a convention
64+
* for "range over <em>all possible values</em> of this ID field" (see {@code Entity.Id.isPartial()}).
65+
* <p><strong>Tip:</strong> Use the {@link tech.ydb.yoj.databind.converter.NotNullColumn} annotation if you only need to overide
66+
* {@code Column.notNull} to {@code true}.
67+
*
68+
* @see <a href="https://github.com/ydb-platform/yoj-project/issues/149">#149</a>
69+
*/
70+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
71+
boolean notNull() default false;
72+
6173
/**
6274
* Determines whether the {@link FieldValueType#COMPOSITE composite field} will be:
6375
* <ul>

‎databind/src/main/java/tech/ydb/yoj/databind/schema/Schema.java‎

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,15 @@ protected Schema(JavaField subSchemaField, @Nullable NamingStrategy parentNaming
128128
this.fields = subSchemaField.fields.stream().map(this::newRootJavaField).toList();
129129
} else {
130130
if (subSchemaField.getCustomValueTypeInfo() != null) {
131-
var dummyField = new JavaField(new DummyCustomValueSubField(subSchemaField), subSchemaField, __ -> true);
131+
// Even if custom value type is a record/POJO/... that contains subfields, we treat it as a flat single-column value
132+
// because that's what a custom value type's ValueConverter returns: a single value fit for a database column.
133+
// (Remember, we do not allow ValueConverter.toColumn() to return a COMPOSITE value or a value of a custom value type)
134+
var dummyField = new JavaField(
135+
new DummyCustomValueSubField(subSchemaField),
136+
subSchemaField,
137+
__ -> true,
138+
this::isRequiredField
139+
);
132140
dummyField.setName(subSchemaField.getName());
133141
this.fields = List.of(dummyField);
134142
} else {
@@ -188,11 +196,11 @@ name, getType(), fieldPath)
188196
columns.add(field.getName());
189197
}
190198
outputIndexes.add(Index.builder()
191-
.indexName(name)
192-
.fieldNames(List.copyOf(columns))
193-
.unique(index.type() == GlobalIndex.Type.UNIQUE)
194-
.async(index.type() == GlobalIndex.Type.GLOBAL_ASYNC)
195-
.build());
199+
.indexName(name)
200+
.fieldNames(List.copyOf(columns))
201+
.unique(index.type() == GlobalIndex.Type.UNIQUE)
202+
.async(index.type() == GlobalIndex.Type.GLOBAL_ASYNC)
203+
.build());
196204
}
197205
return outputIndexes;
198206
}
@@ -250,7 +258,7 @@ private static List<tech.ydb.yoj.databind.schema.Changefeed> collectChangefeeds(
250258
}
251259

252260
private JavaField newRootJavaField(@NonNull ReflectField field) {
253-
return new JavaField(field, null, this::isFlattenable);
261+
return new JavaField(field, null, this::isFlattenable, this::isRequiredField);
254262
}
255263

256264
private JavaField newRootJavaField(@NonNull JavaField javaField) {
@@ -289,6 +297,18 @@ protected boolean isFlattenable(ReflectField field) {
289297
return false;
290298
}
291299

300+
/**
301+
* @param field field
302+
* @return {@code true} if this field is required; {@code false} otherwise
303+
*/
304+
protected boolean isRequiredField(ReflectField field) {
305+
var column = field.getColumn();
306+
if (column != null) {
307+
return column.notNull();
308+
}
309+
return false;
310+
}
311+
292312
public final Class<T> getType() {
293313
return type;
294314
}
@@ -493,22 +513,28 @@ public static final class JavaField {
493513
private final FieldValueType valueType;
494514
@Getter
495515
private final boolean flattenable;
516+
517+
private final boolean required;
518+
496519
@Getter
497520
private String name;
498521
@Getter
499522
private String path;
500523

501524
private final List<JavaField> fields;
502525

503-
private JavaField(ReflectField field, JavaField parent, Predicate<ReflectField> isFlattenable) {
526+
private JavaField(ReflectField field, JavaField parent, Predicate<ReflectField> isFlattenable, Predicate<ReflectField> isRequired) {
504527
this.field = field;
505528
this.parent = parent;
506529
this.flattenable = isFlattenable.test(field);
530+
531+
this.required = (parent != null && parent.required) || isRequired.test(field);
532+
507533
this.path = parent == null ? field.getName() : parent.getPath() + PATH_DELIMITER + field.getName();
508534
this.valueType = field.getValueType();
509535
if (valueType.isComposite()) {
510536
this.fields = field.getChildren().stream()
511-
.map(f -> new JavaField(f, this, isFlattenable))
537+
.map(f -> new JavaField(f, this, isFlattenable, isRequired))
512538
.toList();
513539

514540
if (flattenable && isFlat()) {
@@ -523,6 +549,7 @@ private JavaField(JavaField javaField, JavaField parent) {
523549
this.field = javaField.field;
524550
this.parent = parent;
525551
this.flattenable = javaField.flattenable;
552+
this.required = javaField.required;
526553
this.name = javaField.name;
527554
this.path = javaField.path;
528555
this.valueType = javaField.valueType;
@@ -537,7 +564,7 @@ private JavaField(JavaField javaField, JavaField parent) {
537564
* If the {@link Column} annotation is present, the field {@code dbType} may be used to
538565
* specify the DB column type.
539566
*
540-
* @return the DB column type for data binding if specified, {@code null} otherwise
567+
* @return the DB column type for data binding if specified, {@link DbType#DEFAULT} otherwise
541568
* @see Column
542569
*/
543570
public DbType getDbType() {
@@ -776,6 +803,27 @@ public <J, C extends Comparable<? super C>> CustomValueTypeInfo<J, C> getCustomV
776803
return (CustomValueTypeInfo<J, C>) field.getCustomValueTypeInfo();
777804
}
778805

806+
/**
807+
* @return {@code true} if the database column does accept {@code NULL}; {@code false} otherwise
808+
* @see Column#notNull()
809+
* @see #isRequired()
810+
* @see <a href="https://github.com/ydb-platform/yoj-project/issues/149">#149</a>
811+
*/
812+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
813+
public boolean isOptional() {
814+
return !required;
815+
}
816+
817+
/**
818+
* @return {@code true} if the database column does not accept {@code NULL}; {@code false} otherwise
819+
* @see Column#notNull()
820+
* @see <a href="https://github.com/ydb-platform/yoj-project/issues/149">#149</a>
821+
*/
822+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
823+
public boolean isRequired() {
824+
return required;
825+
}
826+
779827
@Override
780828
public String toString() {
781829
return getType().getTypeName() + " " + field.getName();

‎databind/src/test/java/tech/ydb/yoj/databind/schema/naming/BadMetaAnnotatedEntity.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package tech.ydb.yoj.databind.schema.naming;
2+
3+
import tech.ydb.yoj.databind.converter.NotNullColumn;
4+
import tech.ydb.yoj.databind.converter.ObjectColumn;
5+
6+
public record BadMetaAnnotatedEntity(
7+
Id id,
8+
9+
@ObjectColumn
10+
@NotNullColumn
11+
Key key
12+
) {
13+
public record Key(String parent, long timestamp) {
14+
}
15+
16+
public record Id(String value) {
17+
}
18+
}

‎databind/src/test/java/tech/ydb/yoj/databind/schema/naming/MetaAnnotatedEntity.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package tech.ydb.yoj.databind.schema.naming;
2+
3+
import tech.ydb.yoj.databind.converter.NotNullColumn;
4+
import tech.ydb.yoj.databind.converter.ObjectColumn;
5+
6+
public record MetaAnnotatedEntity(
7+
@NotNullColumn
8+
Id id,
9+
10+
@ObjectColumn
11+
Key key
12+
) {
13+
public record Key(String parent, long timestamp) {
14+
}
15+
16+
public record Id(String value) {
17+
}
18+
}

‎databind/src/test/java/tech/ydb/yoj/databind/schema/naming/MetaAnnotationTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package tech.ydb.yoj.databind.schema.naming;
2+
3+
import org.junit.Test;
4+
import tech.ydb.yoj.databind.FieldValueType;
5+
import tech.ydb.yoj.databind.schema.ObjectSchema;
6+
7+
import static org.assertj.core.api.Assertions.assertThat;
8+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
9+
10+
public class MetaAnnotationTest {
11+
@Test
12+
public void basicMetaAnnotation() {
13+
var schema = ObjectSchema.of(MetaAnnotatedEntity.class);
14+
15+
var idField = schema.getField("id");
16+
assertThat(idField.isRequired()).isTrue();
17+
18+
var keyField = schema.getField("key");
19+
assertThat(keyField.getValueType()).isEqualTo(FieldValueType.OBJECT);
20+
21+
var idColumn = idField.getField().getColumn();
22+
assertThat(idColumn).isNotNull();
23+
assertThat(idColumn.flatten()).isTrue();
24+
assertThat(idColumn.notNull()).isTrue();
25+
26+
var keyColumn = keyField.getField().getColumn();
27+
assertThat(keyColumn).isNotNull();
28+
assertThat(keyColumn.flatten()).isFalse();
29+
assertThat(keyColumn.notNull()).isFalse();
30+
}
31+
32+
@Test
33+
public void multipleAnnotationsNotAllowed() {
34+
assertThatIllegalArgumentException().isThrownBy(() -> ObjectSchema.of(BadMetaAnnotatedEntity.class));
35+
}
36+
}

‎repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/client/YdbSchemaOperations.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,11 @@ public void createTable(String name, List<EntitySchema.JavaField> columns, List<
100100
.orElseThrow(() -> new CreateTableException(String.format("Can't create table '%s'%n"
101101
+ "Can't find yql primitive type '%s' in YDB SDK", name, yqlType)));
102102
ValueProtos.Type typeProto = ValueProtos.Type.newBuilder().setTypeId(yqlType).build();
103-
builder.addNullableColumn(c.getName(), YdbConverter.convertProtoPrimitiveTypeToSDK(typeProto));
103+
if (c.isOptional()) {
104+
builder.addNullableColumn(c.getName(), YdbConverter.convertProtoPrimitiveTypeToSDK(typeProto));
105+
} else {
106+
builder.addNonnullColumn(c.getName(), YdbConverter.convertProtoPrimitiveTypeToSDK(typeProto));
107+
}
104108
});
105109
List<String> primaryKeysNames = primaryKeys.stream().map(Schema.JavaField::getName).collect(toList());
106110
builder.setPrimaryKeys(primaryKeysNames);
@@ -220,8 +224,9 @@ public Table describeTable(String name, List<EntitySchema.JavaField> columns, Li
220224
.map(c -> {
221225
String columnName = c.getName();
222226
String simpleType = YqlType.of(c).getYqlType().name();
227+
boolean isNotNull = c.isRequired();
223228
boolean isPrimaryKey = primaryKeysNames.contains(columnName);
224-
return new Column(columnName, simpleType, isPrimaryKey);
229+
return new Column(columnName, simpleType, isPrimaryKey, isNotNull);
225230
})
226231
.toList();
227232
List<Index> ydbIndexes = indexes.stream()
@@ -339,8 +344,9 @@ private Table describeTableInternal(String path) {
339344
.map(c -> {
340345
String columnName = c.getName();
341346
String simpleType = safeUnwrapOptional(c.getType()).toPb().getTypeId().name();
347+
boolean isNotNull = isNotNull(c.getType());
342348
boolean isPrimaryKey = table.getPrimaryKeys().contains(columnName);
343-
return new Column(columnName, simpleType, isPrimaryKey);
349+
return new Column(columnName, simpleType, isPrimaryKey, isNotNull);
344350
})
345351
.toList(),
346352
table.getIndexes().stream()
@@ -356,6 +362,17 @@ private Type safeUnwrapOptional(Type type) {
356362
return type.getKind() == Type.Kind.OPTIONAL ? type.unwrapOptional() : type;
357363
}
358364

365+
private boolean isNotNull(Type type) {
366+
if (type.getKind() == Type.Kind.VOID || type.getKind() == Type.Kind.NULL) {
367+
// This should never happen: Both Void and Null type can only have NULL as their value, having such columns is pointless.
368+
throw new IllegalStateException("Void and Null types should never be used for columns");
369+
}
370+
371+
// Optional<...> explicitly allows for NULL, other kinds should be NOT NULL by default
372+
// (incl. Lists, Structs, Tuples, Variants are not supported as columns (yet?) but they can be...)
373+
return type.getKind() != Type.Kind.OPTIONAL;
374+
}
375+
359376
public void removeTablespace() {
360377
removeTablespace(tablespace);
361378
}
@@ -478,6 +495,7 @@ public static class Column {
478495
String name;
479496
String type;
480497
boolean primary;
498+
boolean notNull;
481499
}
482500

483501
@Value

‎repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/compatibility/YdbSchemaCompatibilityChecker.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,11 @@ private String makeDropColumn(YdbSchemaOperations.Table table, YdbSchemaOperatio
274274
}
275275

276276
private String makeAddColumn(YdbSchemaOperations.Table table, YdbSchemaOperations.Column c) {
277+
if (c.isNotNull()) {
278+
throw new IllegalArgumentException("Trying to add a NOT NULL column `" + c.getName() + "` but YDB does not support adding "
279+
+ "NOT NULL columns to existing tables, even with a DEFAULT value");
280+
}
281+
277282
if (config.useBuilderDDLSyntax) {
278283
return "DDLQuery.addColumn(" + builderDDLTableNameLiteral(table) + ", " +
279284
javaLiteral(c.getName()) + ", " +
@@ -302,7 +307,7 @@ private static String builderDDLIndexes(YdbSchemaOperations.Table table) {
302307

303308
private static String builderDDLColumns(YdbSchemaOperations.Table table) {
304309
return table.getColumns().stream()
305-
.map(c -> "\t\t.addNullableColumn(" + javaLiteral(c.getName()) + ", " +
310+
.map(c -> "\t\t.add" + (c.isNotNull() ? "NotNull" : "Nullable") + "Column(" + javaLiteral(c.getName()) + ", " +
306311
typeToDDL(c.getType()) + ")\n")
307312
.collect(joining(""));
308313
}
@@ -335,7 +340,7 @@ private static String typeToDDL(String type) {
335340

336341
private static String columns(YdbSchemaOperations.Table table) {
337342
return table.getColumns().stream()
338-
.map(c -> "\t`" + c.getName() + "` " + c.getType())
343+
.map(c -> "\t`" + c.getName() + "` " + c.getType() + (c.isNotNull() ? " NOT NULL" : ""))
339344
.collect(joining(",\n"));
340345
}
341346

@@ -352,13 +357,13 @@ private static String indexes(YdbSchemaOperations.Table table) {
352357
return "\n";
353358
}
354359
return ",\n" + indexes.stream()
355-
.map(idx -> "\t" + indexStatement(idx))
356-
.collect(Collectors.joining(",\n")) + "\n";
360+
.map(idx -> "\t" + indexStatement(idx))
361+
.collect(Collectors.joining(",\n")) + "\n";
357362
}
358363

359364
private static String indexStatement(YdbSchemaOperations.Index idx) {
360365
return String.format("INDEX `%s` GLOBAL %sON (%s)",
361-
idx.getName(), idx.isUnique() ? "UNIQUE " : idx.isAsync() ? "ASYNC " : "", indexColumns(idx.getColumns()));
366+
idx.getName(), idx.isUnique() ? "UNIQUE " : idx.isAsync() ? "ASYNC " : "", indexColumns(idx.getColumns()));
362367
}
363368

364369
private static String indexColumns(List<String> columns) {
@@ -413,7 +418,7 @@ private void makeMigrationTableIndexInstructions(YdbSchemaOperations.Table from,
413418
.collect(toMap(YdbSchemaOperations.Index::getName, Function.identity()));
414419

415420
Function<YdbSchemaOperations.Index, String> createIndex = i ->
416-
String.format("ALTER TABLE `%s` ADD %s;", to.getName(), indexStatement(i));
421+
String.format("ALTER TABLE `%s` ADD %s;", to.getName(), indexStatement(i));
417422

418423
Function<YdbSchemaOperations.Index, String> dropIndex = i ->
419424
String.format("ALTER TABLE `%s` DROP INDEX `%s`;", from.getName(), i.getName());
@@ -461,9 +466,16 @@ private String columnDiff(YdbSchemaOperations.Column column, YdbSchemaOperations
461466
if (column.isPrimary() != newColumn.isPrimary()) {
462467
return "primary_key changed: " + column.isPrimary() + " --> " + newColumn.isPrimary();
463468
}
469+
if (column.isNotNull() != newColumn.isNotNull()) {
470+
return "nullability changed: " + nullabilityStr(column) + " --> " + nullabilityStr(newColumn);
471+
}
464472
return "type changed: " + column.getType() + " --> " + newColumn.getType();
465473
}
466474

475+
private String nullabilityStr(YdbSchemaOperations.Column column) {
476+
return column.isNotNull() ? "NOT NULL" : "NULL";
477+
}
478+
467479
private boolean containsPrefix(String globalName, Set<String> prefixes) {
468480
if (prefixes.isEmpty()) {
469481
return false;

0 commit comments

Comments
 (0)