Reusage schemas fix #1252
Reusage schemas fix #1252
Conversation
…king. Changed behavior for nested schema comparison when strictlyEqualNestedSchemas == true
…n change state upon recursive calls
Jolanrensen
force-pushed
the
reusage-schemas-fix
branch
from
b751010 to
a0c3f2e
Compare
| public fun compare(other: DataFrameSchema, strictlyEqualNestedSchemas: Boolean = false): CompareResult | ||
| public fun compare( | ||
| other: DataFrameSchema, | ||
| comparisonMode: ComparisonMode = STRICT_FOR_NESTED_SCHEMAS, |
There was a problem hiding this comment.
i think LENIENT should be default for more visibility - easier to see where our "special" codegen mode STRICT_FOR_NESTED_SCHEMAS handling is used
| internal fun compareStrictlyEqualNestedSchemas(other: ColumnSchema): CompareResult = compare(other, true) | ||
|
|
||
| private fun compare(other: ColumnSchema, strictlyEqualNestedSchemas: Boolean): CompareResult { | ||
| public fun compare(other: ColumnSchema, comparisonMode: ComparisonMode = STRICT_FOR_NESTED_SCHEMAS): CompareResult { | ||
| if (kind != other.kind) return CompareResult.None | ||
| if (this === other) return CompareResult.Equals | ||
| return when (this) { | ||
| is Value -> compare(other as Value) |
There was a problem hiding this comment.
Seems odd that comparison mode is not used here. How you tell the difference between nullable and non-nullable column?
There was a problem hiding this comment.
That's true, I had it at one point, but it wasn't in the implementation before... Let's see what breaks if I add it back
There was a problem hiding this comment.
Ah, the same behavior was achieved by if (comparison != Equals && comparisonMode == STRICT) None else comparison in the other file, but I improved it now so ColumnSchema.Value now also has a comparisonMode argument and the "strictness increase" is better explained.
Sorry, I just had no clue what was going on before refactoring, let alone debug how it should behave. Hopefully the new approach expresses the intention behind the code better :) |
Fixes #1222 by replacing the
strictlyEqualNestedSchemasparameter by a more explicitComparisonMode.Comparing two schemas in
LENIENTmode can returnIsSuper,IsDerived,IsEqual, orNone.Compating in
STRICTmode can only returnIsEqualorNone, because the schema's need to exactly match.STRICT_FOR_NESTED_SCHEMASworks inLENIENTmode for the top-level, butSTRICTfor nested schemas. This is often used in Jupyter notebooks, to prevent nested types from extending each other and thus avoid a potential comparison explosion. (There could be a lot of nested types)Also, added documentation everywhere
Requires tiny patch in the compiler plugin