Clarify that schema of property with DataFrame<T>? type is not a FrameColumn

[koperagen]

This makes extracted schema represent runtime more accurately
With compiler plugin enabled, check this:

val df = dataFrameOf("a" to columnOf(List(4) {
    dataFrameOf(
        "c" to columnOf(2)
    )
}))
    .addId()

val res = df
    .convert { a }.with { it.takeIf { index() % 2 == 0 } }
    .add("f1") { a }

println(res.compileTimeSchema())

Reproduced by extractNullableDataFrameSchema test

Before, schema would lie to us that a and f1 are FrameColumns (and has no nulls):

id: Int
a: *
    c: Int?
f1: *
    c: Int?

After:

id: Int
a: DataFrame<`FieldkindKt$main$df$2$A_651`>?
f1: DataFrame<`FieldkindKt$main$df$2$A_651`>?

Result is arguably not much better, because now suddenly we have not expanded structural dataframe type in the schema. In this case it is also anonymous and cannot be "materialized" by either generateCode or recent compiler warnings utilities.
Resolving this usability problem will be our next step. We can either, idk, re-define operations in a way that nulls automatically replaced with empty frames, or fix codegen/schema/model to reflect possible DataFrame values better

Regarding changes made in this PR:

1. toDataFrame is expected to behave exactly as before, but it requires additional condition
2. convertTo used to be able to convert FrameColumn<Schema> to a ValueColumn<DataFrame<Schema>?> (which is valid operation) due to MarkersExtractor.get wrongly interpreting DataFrame<Schema>? as FrameColumn. I added branch that explicitly allows such pre-existing convertTo

fixes #1498

[Jolanrensen]

What does a DataFrame with a nullable type mean? DataFrame<Location> makes sense: you have two columns "name" and "gps". But I don't know how to interpret DataFrame<Location?>.

[Jolanrensen]

should we prohibit such types? Or explicitly allow them (changing "name" and "gps" to nullable versions and fill them with nulls, like in our json reader)

[koperagen]

Iterable<T>.toDataFrame(): DataFrame<T>
In this case T just happens to be Location?
Let's say, with compiler plugin enabled this will not happen

[Jolanrensen]

Oh, that's indeed exactly what happens, I see.

[Jolanrensen]

Indeed, the compiler plugin will come up with a new type. But it might be worth to define this behavior as a rule (in docs, tests, etc).

• DataFrame<T>: T is expected to be not nullable. If it is nullable, all value-columns will become nullable and all frame-columns can contain empty dataframes.

[koperagen]

ok, but as a different issue

[Jolanrensen]

FieldKind.of is a very strange factory function... Is it this way because it's used elsewhere? like in the compiler plugin? :)

[koperagen]

indeed, it is used there

[Jolanrensen]

if you return originalColumn the kind won't have changed to Value, right?

[Jolanrensen]

Plus, if it does change the kind, this might enable converting FrameColumn -> ValueColumn<AnyFrame> which is not allowed

[koperagen]

If originalColumn has no nulls in runtime then creating ValueColumn is a violation and our checks fail
So yes..
It seems to create misalignment one way or another.

[Jolanrensen]

So isn't this step then gonna break regardless? It converts FrameColumn -> ValueColumn
But FrameColumn cannot contain nulls, so it will always need to create a ValueColumn<DataFrame<>> which is not allowed.
I don't think there's conversion possible from FrameColumn -> ValueColumn. Maybe the other way around but only if the nulls are turned into empty dataframes

[koperagen]

Approach that i chose now is keeping runtime column "narrower" than compile time - it's usually a valid approach. So, even if target type is DataFrame<T>? we just keep runtime FrameColumn where possible
let's say convertTo<...>.dropNulls { nullableDataFrameCol into "frameCol" } => valid frameCol
Problem is that it's turns out not only narrow, but also of a different kind ...

[Jolanrensen]

I think logic like that will only complicate things more... Can we try to keep the rules the same everywhere?

• DataColumn<DataFrame<>?> must always be a ValueColumn, in compile time as well as runtime
• DataColumn<DataFrame<>> must always be a FrameColumn, in both cases too

This will make it much easier for us and to explain to users. So if the target type is DataColumn<DataFrame<>?>, we turn it into a value column, if it'sDataColumn<DataFrame<>>, it becomes a frame column.

convertTo<...>.dropNulls { nullableDataFrameCol into "frameCol" } => valid frameCol

This will then also work correctly, right? because nullableDataFrameCol: DataFrame<>? so it must have been a ValueColumn, then after dropNulls, it automatically turns into a FrameColumn
(If this doesn't work for dropNulls yet, that should probably be fixed. It works correctly for update)

[koperagen]

DataColumn<DataFrame<>?> must always be a ValueColumn, in compile time as well as runtime - existing check prevents that
If field is DataFrame<>? and in runtime there were no nulls, according to this logic toDataFrame (changing topic from convertTo because problem originates there) should create a ValueColumn - but we forbid that

[Jolanrensen]

I see now... that's indeed a problem. In that case DataColumn<DataFrame<>?> can indeed be a FrameColumn in runtime too... Alright:

• DataColumn<DataFrame<>> must always be a FrameColumn in runtime
• DataColumn<DataFrame<>?> is likely a ValueColumn but can also be a FrameColumn in runtime because it may not contain nulls.

I suppose from the convertToImpl this means we should treat ColumnKind a bit more loosely when dealing with nullable DataFrame types. But only for those. If we're explicit enough about this case it should be fine I think :)

[koperagen]

Yes, this matches my understanding of the situation.

I suppose from the convertToImpl this means we should treat ColumnKind a bit more loosely when dealing with nullable DataFrame types. But only for those. If we're explicit enough about this case it should be fine I think :)

Yeah, this should be resolved.
I see situation like this: we had a bug that used to hide a few design problems. Now the bugs is fixed, but design problems become more apparent. Same as before having DataFrame? is error prone - before it could lead to NPE, now it leads to inconsistency if specific column selectors are used, for example, or when code for schema is generated.
So i'd say DataFrame<T>? in @DataSchema is not recommended, DataFrame<T>? value in add() { } can appear if needed but has usability issues around schema printing and codegen