Skip to content

Commit b852bd9

Browse files
nvamelichevnvamelichev
committed
#149: Support NOT NULL columns via @Column(notNull=true)
Defaulting ID fields to required (`NOT NULL`) can be enabled via a Java system property, `tech.ydb.yoj.repository.db.id.nullability`, which will influence the schema checker (`YdbSchemaCompatibilityChecker`), programmatic table creation via `YdbSchemaOperations`, and YQL code generation in `YqlStatement`s that take into account the `isOptional()` `JavaField` attribute. The new attribute is introduced by this change, so the only supporting code generators initially are the standard `YqlStatement` implementations. The valid System property values are: - `USE_COLUMN_ANNOTATION` (Default): Consider ID fields optional unless annotated with the `@Column(notNull=true, ...)` annotation or the `@NotNullColumn` meta-annotation. - `ALWAYS_NULL`: Force all ID fields to be treated as optional, even though YOJ does **not** allow to save entities with NULL in ID columns. - `ALWAYS_NOT_NULL`: Force all ID fields to be treated as required.
1 parent b26d783 commit b852bd9

File tree

19 files changed

+367
-71
lines changed

19 files changed

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

251259
private JavaField newRootJavaField(@NonNull ReflectField field) {
252-
return new JavaField(field, null, this::isFlattenable);
260+
return new JavaField(field, null, this::isFlattenable, this::isRequiredField);
253261
}
254262

255263
private JavaField newRootJavaField(@NonNull JavaField javaField) {
@@ -288,6 +296,19 @@ protected boolean isFlattenable(ReflectField field) {
288296
return false;
289297
}
290298

299+
/**
300+
* @param field field
301+
* @return {@code true} if this field is required; {@code false} otherwise
302+
*/
303+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
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+
291312
public final Class<T> getType() {
292313
return type;
293314
}
@@ -492,22 +513,28 @@ public static final class JavaField {
492513
private final FieldValueType valueType;
493514
@Getter
494515
private final boolean flattenable;
516+
517+
private final boolean required;
518+
495519
@Getter
496520
private String name;
497521
@Getter
498522
private String path;
499523

500524
private final List<JavaField> fields;
501525

502-
private JavaField(ReflectField field, JavaField parent, Predicate<ReflectField> isFlattenable) {
526+
private JavaField(ReflectField field, JavaField parent, Predicate<ReflectField> isFlattenable, Predicate<ReflectField> isRequired) {
503527
this.field = field;
504528
this.parent = parent;
505529
this.flattenable = isFlattenable.test(field);
530+
531+
this.required = (parent != null && parent.required) || isRequired.test(field);
532+
506533
this.path = parent == null ? field.getName() : parent.getPath() + PATH_DELIMITER + field.getName();
507534
this.valueType = field.getValueType();
508535
if (valueType.isComposite()) {
509536
this.fields = field.getChildren().stream()
510-
.map(f -> new JavaField(f, this, isFlattenable))
537+
.map(f -> new JavaField(f, this, isFlattenable, isRequired))
511538
.toList();
512539

513540
if (flattenable && isFlat()) {
@@ -522,6 +549,7 @@ private JavaField(JavaField javaField, JavaField parent) {
522549
this.field = javaField.field;
523550
this.parent = parent;
524551
this.flattenable = javaField.flattenable;
552+
this.required = javaField.required;
525553
this.name = javaField.name;
526554
this.path = javaField.path;
527555
this.valueType = javaField.valueType;
@@ -536,7 +564,7 @@ private JavaField(JavaField javaField, JavaField parent) {
536564
* If the {@link Column} annotation is present, the field {@code dbType} may be used to
537565
* specify the DB column type.
538566
*
539-
* @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
540568
* @see Column
541569
*/
542570
public DbType getDbType() {
@@ -783,6 +811,27 @@ public <J, C extends Comparable<? super C>> CustomValueTypeInfo<J, C> getCustomV
783811
return (CustomValueTypeInfo<J, C>) field.getCustomValueTypeInfo();
784812
}
785813

814+
/**
815+
* @return {@code true} if the database column does accept {@code NULL}; {@code false} otherwise
816+
* @see Column#notNull()
817+
* @see #isRequired()
818+
* @see <a href="https://github.com/ydb-platform/yoj-project/issues/149">#149</a>
819+
*/
820+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
821+
public boolean isOptional() {
822+
return !required;
823+
}
824+
825+
/**
826+
* @return {@code true} if the database column does not accept {@code NULL}; {@code false} otherwise
827+
* @see Column#notNull()
828+
* @see <a href="https://github.com/ydb-platform/yoj-project/issues/149">#149</a>
829+
*/
830+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/149")
831+
public boolean isRequired() {
832+
return required;
833+
}
834+
786835
@Override
787836
public String toString() {
788837
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)