Skip to content

[SPARK-46448][SQL] InlineTable column should be nullable when converted to Decimal might cause null#44404

Closed
amaliujia wants to merge 4 commits intoapache:masterfrom
amaliujia:resolve_table_nullability
Closed

[SPARK-46448][SQL] InlineTable column should be nullable when converted to Decimal might cause null#44404
amaliujia wants to merge 4 commits intoapache:masterfrom
amaliujia:resolve_table_nullability

Conversation

@amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Dec 18, 2023

What changes were proposed in this pull request?

InlineTable column can also be nullable when there is type conversion to Decimal and it is not safe to be converted to null. So this PR updates the column nullability to true when there is type conversion to Decimal and the conversion is safe to make the new type null.

Note that given current implementation for conversion to Decimal, it is not likely null will happen, however because canNullSafeCastToDecimal still could return false so we need a defensive check for that case.

Why are the changes needed?

This is to make the nullability behavior consistent after type conversion in InlineTable.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

UT

Was this patch authored or co-authored using generative AI tooling?

NO

@github-actions github-actions bot added the SQL label Dec 18, 2023
@amaliujia
Copy link
Contributor Author

@cloud-fan

@cloud-fan
Copy link
Contributor

Can you fully fill the PR description?

@amaliujia
Copy link
Contributor Author

@cloud-fan my bad. The save button didn't work on my previous edit.

@amaliujia amaliujia changed the title [SPARK-46448] InlineTable column should be nullable when converted to Decimal might cause null [SPARK-46448][SQL] InlineTable column should be nullable when converted to Decimal might cause null Dec 18, 2023
}

test("SPARK-46448: InlineTable column is nullable if converting to Decimal could be null") {
val plan = sql("SELECT * FROM values(-0.172787979),(533704665545018957788294905796.5)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, so we still get null value at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a defensive change because Cast.canNullSafeCastToDecimal(t, d) still has code to return false. though we know that no null value will happen at the end.

@amaliujia
Copy link
Contributor Author

Closing this PR for now as this may not be needed given that the real value will not be null.

@amaliujia amaliujia closed this Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants