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

[WIP] Allow underscores in varbinary literals #24776

Closed

Conversation

brunokim
Copy link
Contributor

Description

Allow underscores in addition to whitespace within varbinary literals.

Additional context and related issues

This PR has #24774 as a dependency.

This change was a minor point in the discussion of #23682, but I thought it
would be a good starting point to me as a contributor.

Numeric literals allow underscores for readability, and a developer (like me)
may expect that varbinary literals would use the same pattern. This syntax is more
lenient than the one for numbers, ignoring any number of underscores (or whitespace)
in the string.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Allow underscores in varbinary literals ({23682}`23682`)

Varbinary literals ignore any whitespace within the string to increase
readability. Numeric literals allow underscores as digit separators, and
a developer may expect the same affordance when using varbinary.
@mosabua
Copy link
Member

mosabua commented Jan 23, 2025

Could you also look at #24784 and talk to @martint or maybe @kasiafi

@kasiafi
Copy link
Member

kasiafi commented Jan 27, 2025

@brunokim per SQL specification 5.3 <literal>, underscore is illegal in hex literals. Whitespace is legal and Trino supports it. What is the purpose of having two separator characters?

Also, if we aimed at consistency with numeric literals, we should disallow leading, trailing and consecutive underscores.

@brunokim
Copy link
Contributor Author

@brunokim per SQL specification 5.3 <literal>, underscore is illegal in hex literals. Whitespace is legal and Trino supports it. What is the purpose of having two separator characters?

Oh, I don't have access to the specification and didn't know this choice comes from there. This change should be closed, then.

Also, if we aimed at consistency with numeric literals, we should disallow leading, trailing and consecutive underscores.

It's a matter of opinion, for sure, but moot since the standard already addresses it. IMO a binary literal is more like a number than like a string, but a binary object is more like a string than like a number, so I can empathize with these choices.

@brunokim brunokim closed this Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants