[SPARK-59515][PYTHON] Compare leaf type parameters in assertSchemaEqual - #58791
laserninja wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
The code change looks correct and is well tested.
Change. In compare_datatypes_ignore_nullable, the final leaf branch now returns dt1 == dt2 instead of unconditionally True. Previously any two types sharing a typeName() were treated as equal, which silently ignored semantic parameters unrelated to nullability (collation, char/varchar length, time precision, interval field bounds).
Correctness. The contract of "ignore nullable only" is preserved: the only types carrying nullability — array (containsNull), map (valueContainsNull), struct (field.nullable) — are all handled by the recursive branches above the else, so the leaf branch only ever sees atomic/parameterized types that have no nested nullability. DataType.__eq__ compares type + __dict__, so StringType("UTF8_LCASE") != StringType(), CharType(1) != CharType(10), TimeType(3) != TimeType(6), DayTimeIntervalType(0,0) != DayTimeIntervalType(0,3), etc. — exactly the cases the old code missed. decimal keeps its dedicated precision/scale branch.
Tests. Nice coverage — test_parameterized_data_types sweeps 7 type-pairs across 5 wrappings (bare, array element, map key, map value, struct field), asserting DIFFERENT_SCHEMA and that a type stays equal to itself; test_nested_nullability_is_ignored confirms nullability is still ignored (toNullable()) while leaf parameters are compared, and that ignoreNullable=False still raises. SchemaComparisonTests(unittest.TestCase) is the right lightweight base (pure Python, no SparkSession). No new >100-char or non-ASCII lines.
One item before merge: the title is [PYTHON] ... with no JIRA. This is a user-facing behavior change (with ignoreNullable=True, schemas differing only in leaf parameters now raise DIFFERENT_SCHEMA), so it isn't MINOR/TRIVIAL and should have a [SPARK-xxxxx] ticket per contribution guidelines — the description also notes the ASF JIRA is still pending. Please create/link it and update the title. The behavior change itself is the correct fix (the helper should ignore only nullability); worth flagging that user tests relying on the old loose matching may start failing.
e21432c to
96fc3d7
Compare
|
Thanks for the review. Created SPARK-59515, linked it in the description, and added the Jira ID to the title. I also explicitly noted that user tests relying on the previous loose matching of leaf-type parameters may now fail correctly with DIFFERENT_SCHEMA, while recursive nullability remains ignored. No code changes were needed for these review items. |
What changes were proposed in this pull request?
Jira: SPARK-59515
Use data-type equality for leaf types after the existing recursive nullability handling. Add coverage for collations, character lengths, time precision, interval bounds, and nested containers.
Why are the changes needed?
The ignoreNullable comparator accepts remaining types solely because typeName matches. This also ignores semantic parameters unrelated to nullability, such as UTF8_BINARY versus UTF8_LCASE collations.
Does this PR introduce any user-facing change?
Yes. With ignoreNullable=True, schemas with unequal leaf-type parameters now raise DIFFERENT_SCHEMA. Recursive array, map, and struct nullability remains ignored. User tests that relied on the previous loose matching of leaf-type parameters may now fail correctly with DIFFERENT_SCHEMA.
How was this patch tested?
The new regression tests failed before the fix and passed afterward on Python 3.10.11 with NumPy 2.2.6. The focused run passed 2 tests, including parameterized subtests:
Run the commands with the Python virtual environment activated; the local run shared an environment from a sibling checkout. The tests call public Python APIs directly and require no SparkSession. Python compilation, custom-error checks, Ruff lint, and Ruff formatting passed. JVM-backed suites were not run because these fixes are confined to Python-side logic.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-6).