Skip to content

Conversation

@mihailoale-db
Copy link
Contributor

@mihailoale-db mihailoale-db commented Mar 21, 2025

What changes were proposed in this pull request?

I propose that we fix error message when to_timestamp function is called with improper argument types.

Why are the changes needed?

Users are provided with more user facing error instead of an internal one, which is more correct.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added test.

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

No.

@github-actions github-actions bot added the SQL label Mar 21, 2025
e.invalidFormat(checkRes)
}

case parseToTimestamp: ParseToTimestamp if parseToTimestamp.left.dataType != StringType =>
Copy link
Member

Choose a reason for hiding this comment

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

ParseToTimestamp should support numeric, see #35887. cc @HyukjinKwon

And I am not sure that checking parameters of particular expression here in checkAnalysis is right approach. IMHO, ParseToTimestamp should care of them.

Copy link
Member

Choose a reason for hiding this comment

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

Can we do it in checkInputDataTypes or inputTypes in ParseToTimestamp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I just wonder what's the semantics here: should we support NumericTypes here or no? cc @srielau
Per docs, it seems that we should support STRING type only.

Copy link
Member

Choose a reason for hiding this comment

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

I think the problem is that it is a breaking change .. I would leave it to @srielau tho.

@github-actions
Copy link

github-actions bot commented Jul 3, 2025

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jul 3, 2025
@github-actions github-actions bot closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants