Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support LargeUtf8 to Temporal Coercion #8357

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

jayzhan211
Copy link
Contributor

Which issue does this PR close?

Ref #8302

Rationale for this change

There are two reason why I want to support this coercion.

One is that LargeUtf8 to Timestamp is supported in coerced_from. We need the same so I can replace it with string_temporal_coercion
Second is that comment in string_temporal_coercion mentioned LargeUtf8 but the code itself does not.

What changes are included in this PR?

Are these changes tested?

I didnt find coercion test in slt. I put it in scalar.slt for now.

Are there any user-facing changes?

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@github-actions github-actions bot added logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels Nov 29, 2023
}

match_rule(lhs_type, rhs_type).or_else(|| match_rule(rhs_type, lhs_type))
Copy link
Contributor Author

@jayzhan211 jayzhan211 Nov 29, 2023

Choose a reason for hiding this comment

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

without this it will be too hard to check if the code cover one type.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a neat construction 👍

@jayzhan211 jayzhan211 changed the title Support LargeUtf to Temporal Coercion Support LargeUtf8 to Temporal Coercion Nov 29, 2023
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Very cool @jayzhan211 -- thank you for the contribution

}

match_rule(lhs_type, rhs_type).or_else(|| match_rule(rhs_type, lhs_type))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a neat construction 👍

@alamb alamb merged commit a588123 into apache:main Nov 30, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants