☂ type: KType in DataColumnImpl mismatches actual values sometimes

[Jolanrensen]

type: KType in DataColumnImpl mismatches actual values in some cases. This can result in runtime exceptions and makes life difficult attempting to fix #30 or #704 where we assume the type always correctly represents the data.
This issue relates to #701 as well.

To discover these bugs, we can introduce a (debug-only!!) check in DataColumnImpl, like:

private infix fun <T> T?.matches(type: KType) =
    when {
        this == null -> type.isMarkedNullable
        this.isPrimitiveArray -> type.isPrimitiveArray &&
            this!!::class.qualifiedName == type.classifier?.let { (it as KClass<*>).qualifiedName }

        this.isArray -> type.isArray // cannot check the precise type of array
        else -> this!!::class.isSubclassOf(type.classifier as KClass<*>)
    }

init {
    if (DEBUG) {
        require(values.all { it matches type }) {
            val types = values.map { if (it == null) "Nothing?" else it!!::class.simpleName }.distinct()
            "Values of column '$name' have types '$types' which are not compatible given with column type '$type'"
        }
    }
}

At the moment of testing, I can find 8+ breaking tests in :core:

• org.jetbrains.kotlinx.dataframe.io.PlaylistJsonTest#aggregate by column
  • data has types [DataFrameImpl], kType is: org.jetbrains.kotlinx.dataframe.aggregation.AggregateGroupedDsl<org.jetbrains.kotlinx.dataframe.io.PlaylistJsonTest.DataFrameType1>
  • This exception is probably fine to ignore, as AggregateGroupedDsl is a DataFrame
• org.jetbrains.kotlinx.dataframe.samples.api.Analyze#pivotDefault_accessors / #pivotDefault_strings / #pivotDefault_properties
  • data has types [Boolean, Int], kType is: kotlin.Boolean
  • Not really sure what's going on here, something with concatImpl I think
• org.jetbrains.kotlinx.dataframe.samples.api.Modify#customConverters
  • data has types [Nothing?], kType is kotlin.Int
  • Seems to originate from convertToImpl$convertToSchema
• org.jetbrains.kotlinx.dataframe.samples.api.Modify#implode
  • data has types [DataFrameImpl, Nothing?], kType is: org.jetbrains.kotlinx.dataframe.DataFrame<*>, not nullable
  • Seems to originate from implodeImpl where a null is put in a FrameColumn
• org.jetbrains.kotlinx.dataframe.testSets.person.DataFrameTests#convertTo
  • data has type: [Nothing?], kType is: kotlin.Int
  • From convertToImpl$convertToSchema too
• org.jetbrains.kotlinx.dataframe.testSets.person.DataFrameTreeTests#merge rows into table
  • data has types [DataFrameImpl, Nothing?], kType is: org.jetbrains.kotlinx.dataframe.DataFrame<*>, not nullable
  • Seems to originate from implodeImpl too where a null is put in a FrameColumn

Edit: running it afresh (clean pull of master with check) I get 15 failing tests.

There is also an exception in :dataframe-jdbc: #701

[Jolanrensen]

7/15 tests are fixed by #727

Next are the pivot tests with 0 and true in Boolean columns.
I suspect this is due to default() taking Any? and the pivot implementation not re-inferring the types of the columns after filling in a null with the given default in: df.pivot { city }.groupBy { name }.default(0).min().

[Jolanrensen]

After merging #734 there are just 5 failing tests left:

[Jolanrensen]

Maybe we could use https://github.com/gmazzo/gradle-buildconfig-plugin to set a DEBUG flag and check these types when running tests

[Jolanrensen]

#796 adds the ability to set -Pkotlin.dataframe.debug=true and run the checks for this issue :)

[Jolanrensen]

@zaleslaw After #796 is merged, I´d recommend running the JDBC tests again in debug mode.

I now have 15 failing tests just from that module. They contain all sorts of Char/String, Double/Float, and Int/Short mismatches. These will most likely result in runtime exceptions on the user's side if left untreated.

[Jolanrensen]

One of the tests triggeres in convertToImpl. It seems like a temporary null-filled column is created even though the column type might be not-nullable. This needs a small workaround but probably would not have caused user-facing bugs.

Edit: fixed by #800

[Jolanrensen]

The last two main tests are fixed by #801 and #803.

To solve this issue, we just need to:

• Fix the 15 new failing JDBC tests: JDBC KType mismatches in "debug" mode #804
• Make TC run tests with -Pkotlin.dataframe.debug=true

[zaleslaw]

Please, @Jolanrensen, create a ticket with the list of these JDBC tests, include in 0.14 milestone and assign on me

[Jolanrensen]

Attempt to work with assert() and the -ea JVM flag as it does the same :)

Conclusion: this is not an option. It also runs when DataFrame is used in a user-project test.

[Jolanrensen]

Debug mode is now enabled on TeamCity! Thanks for the help fixing the issues @zaleslaw :)