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

Feat: Add support for REVERSE #4615

Closed
wants to merge 1 commit into from
Closed

Feat: Add support for REVERSE #4615

wants to merge 1 commit into from

Conversation

pruzko
Copy link
Contributor

@pruzko pruzko commented Jan 15, 2025

REVERSE(x) reverses the string. Aside from SQLite, I am not aware of any engine that does not support it.

Oracle
MySQL
Postgres
TSQL

@VaggelisD
Copy link
Collaborator

Generally we'd want to introduce a new expression if there's a need to transpile between dialects with different semantics and/or args; For this case, it seems like we have REVERSE(x) across the board, so exp.Anonymous would be fine imo.

As a note, there are other dialects that support REVERSE like Snowflake, Spark/DB, BQ, Presto/Trino etc; If we introduced exp.Reverse without checking them first we could have broken their roundtrips, luckily they all accept a single input afaict.

With that in mind, the only use case I see for exp.Reverse would be for the type annotator.

@georgesittas
Copy link
Collaborator

Agreed with Vaggelis, I don't see what this PR currently achieves. Closing it for now but happy to continue discussing.

@pruzko
Copy link
Contributor Author

pruzko commented Jan 15, 2025

I thought you're covering everything so that you can ast.find(exp.Reverse) but yeah, there is not point in adding this. Good to know tho.

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