Skip to content

[SPARK-44271][SQL] Move default values functions from StructType to ResolveDefaultColumns#41820

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

[SPARK-44271][SQL] Move default values functions from StructType to ResolveDefaultColumns#41820
amaliujia wants to merge 4 commits intoapache:masterfrom
amaliujia:clean_up_left_errors

Conversation

@amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Jul 2, 2023

What changes were proposed in this pull request?

Move default values functions from StructType to ResolveDefaultColumns.

Why are the changes needed?

To simply DataType interface.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing test

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

R: @cloud-fan

@amaliujia amaliujia changed the title [SPARK-44271][SQL] Move util functions from DataType to ResolveDefaultColumns [SPARK-44271][SQL] Move default values functions from StructType to ResolveDefaultColumns Jul 2, 2023
@amaliujia amaliujia force-pushed the clean_up_left_errors branch from b82a0c6 to 2e9daec Compare July 4, 2023 04:30
Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra newline?

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not appear to cache the results of each method, as this comment suggests. The previous code did this with a lazy val. Can you update the comment to mention that these results are not cached and that callers should take care to avoid calling them repeatedly in tight loops during query execution (e.g. per-row) and instead prefer to initialize them as infrequently as possible, preferably only once?

@amaliujia amaliujia force-pushed the clean_up_left_errors branch from 79483ad to fe710eb Compare July 8, 2023 23:53
@hvanhovell
Copy link
Contributor

Merging.

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.

4 participants