Skip to content

Conversation

@nicktobey
Copy link
Contributor

These are a couple more functions whose inputs aren't being unwrapped. If the inputs come from a column with a TEXT type, then they won't be converted to strings and the functions will fail. This PR fixes this by making sure that the inputs are converted.

We missed this previously because although we have tests for these functions, none of them tested getting the inputs from TEXT columns of a table.

Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

Looks good. I'm not sure why those blocks are there, but you can ship it if you want them there for some reason I'm not seeing.


if fromStr.(string) == "" {
return str, nil
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the additional block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The blocks are there so that I can redeclare fromStr et al as strings instead of interface{}. Without the block I'd need to pick new names for the unwrapped versions, which would be noisy.

}

return padString(str.(string), length.(int64), padStr.(string), p.padType)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

again on the block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above.

@nicktobey nicktobey merged commit 4ae8110 into main May 2, 2025
7 of 8 checks passed
@nicktobey nicktobey deleted the nicktobey/unwrap branch May 2, 2025 21:49
@nicktobey nicktobey restored the nicktobey/unwrap branch May 2, 2025 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants