doanduyhai / Achilles

An advanced Java Object Mapper/Query DSL generator for Cassandra
http://achilles.archinnov.info
Apache License 2.0
241 stars 92 forks source link

@Column default value #348

Closed vrudikov closed 5 years ago

vrudikov commented 5 years ago

Hi, @doanduyhai

Glad to hear you again :)

how do you think - is there any sense in defaultValue property for @Column annotation

Sometimes when Cassandra has null in a column we can get NPE for primitive type:

java.lang.NullPointerException
    at info.archinnov.achilles.generated.meta.entity.Order_AchillesMeta.lambda$static$81(Order_AchillesMeta.java:319)
    at info.archinnov.achilles.internals.metamodel.AbstractProperty.decodeField(AbstractProperty.java:245)
    at info.archinnov.achilles.internals.metamodel.AbstractEntityProperty.lambda$createEntityFrom$49(AbstractEntityProperty.java:245)
doanduyhai commented 5 years ago

By definition all values in Cassandra can be null. Therefore on the client side you need to use Object type and not primitive types.

Now, to cope with NPE, I have introduced support for Java 8 Optional:

@Column
private Optional<Integer>  value;

But the idea of defaultValue does also make sense. Let me evaluate how hard it is to implement it in Achilles

vrudikov commented 5 years ago

A-W-E-S-O-M-E!

So the column annotation will give us possibility to set defaultValue for nulled Cassandra value in Achilles Runtime

@Column(name = "col1", defaultValue = 0.0)
private double value;

So If we have defaultValue maybe we can easily use primitives now?

doanduyhai commented 5 years ago

There is one detail to think about: should the defaultValue apply to mutations (if null in Java --> write defaultValue in Cassandra) or should it only apply to reads (if null in Cassandra --> default value in Java) ?

I'm strongly in favor of only implementing defaultValue only for reads. I don't like the idea of writing automagically default values into Cassandra in case of null in Java because:

1) we all know that with Java beans convention, every object not initialised will be null so with defaultValue enabled on mutations you'll end up writing a lot of values into Cassandra. I hate magic behaviors like this 2) this feature plays agains the existing insert strategy NON_NULL_FIELDS and I don't want to introduce regressions/bugs with an old long existing feature 3) users can also enforce functional rules on their Java beans by applying default values on them before passing the beans to Achilles

vrudikov commented 5 years ago

I'm strongly in favor of only implementing defaultValue only for reads

Agree with you. I think it should be limited to reads only

But how can you restrict it for READ operations only?

doanduyhai commented 5 years ago

To implement this feature for reads only, below are the steps:

1) At compile time, parse the @Column annotation and retrieve the defaultValue attribute 2) Store the default value in meta data 3) At runtime, on reads, if value is null, read default value from meta data

Now there is a big issue. Indeed for annotation attributes, the value of the attribute can only be one of those types:

See Java language specs here: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1

So even if we implement defaultValue, we're quite restricted in the type support, mainly if your column is of the following Cassandra type, it will not work:

Because of the impossibility of supporting defaultValue for those types, it will make the feature inconsistent and I really hate introducing new inconsistent features into Achilles.

The current status is there is a technical road block to support defaultValue for reads, unless you have a better idea to provide default values at compile time not using annotation ....

iweawer commented 5 years ago

Hi @doanduyhai (@vrudikov is my collegue). I can propose a bit different approach. The separate function which returns default values annotated with special annotation

@ColumnDefaultValueProvider
public Object getDefaultValue(String columnName) {
   switch(columnName) {
        case "price":
            return 0;
        // case ...
    }
}

or it can be separate function for each column

@ColumnDefaultValue("price")
public double defaultPrice() {
  return 0.0
}

So logic will be following

  1. At compile time, parse the @ColumnDefaultValueProvider or @ColumnDefaultValue annotations
  2. Store functions in meta data
  3. At runtime, on reads, if value is null, call the function saved in meta data to get default value

In this approach you can provide default value for any type. It also possible to implement dynamic default value.

doanduyhai commented 5 years ago

Sounds good, I would also add type checking for the return type of @ColumnDefaultValue that should match the type of the column it points to.

Be aware that this feature requires a lot of work (parsing source code at compile-time is not trivial) so don't expect this feature to be implemented right away

vrudikov commented 5 years ago

Sure. We don't expect that. We know it's hard. I had experience with Achilles code complexity and I broke my leg there ๐Ÿ— ๐Ÿ˜„

doanduyhai commented 5 years ago

Closing

A very simple solution is to use Interceptor on POST_LOAD to set default values to the columns of your choice. This solution is way more elegant than adding another layer of complexity into the framewirk