Skip to content

Conversation

@Newbie012
Copy link
Contributor

I'm aware that this PR won't be accepted until I add the appropriate tests and documentation. I first want to make sure that this feature is acceptable to the core maintainers.

My apologies for not writing a separate issue for that. If you believe sql tags should not exist in TypeORM, I will understand. Although, it would be harder for other tools (such as SafeQL) to integrate with TypeORM.


Description of change

Adds a new tagged template to classes that execute raw SQL queries and return raw database results. The template takes a template string and an array of values as input and builds a SQL query using the appropriate parameter strategy based on

While SQL tags can be misleading to some people regarding SQL injections, they are, in fact, safer than splitting the query from the variables. With tagged template, you simply can't go wrong with accidently injecting unsafe values from variables due to the way of how tagged templates work.

As a plus, reading the query with the variables in place adds more clarity rather than trying to figure out what is the $23 parameter.

The new sql tag will split the templates from the expressions, parameterize the statement, and then map the statement and the variables back to the query method. (tag -> { query, variables } -> query method).

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • [N/A] This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

Adds a new tagged template  to  and  classes that executes raw SQL queries and returns raw database results. The template takes a template string and an array of values as input and builds a SQL query using the appropriate parameter strategy based on
@fingeromer
Copy link

This is super helpful for us! Thanks @Newbie012

@fingeromer
Copy link

any new with this? will be helpful for us

@fingeromer
Copy link

Hi team, any new with this PR? we would love to have this update in TypeORM

Copy link

@hinogi hinogi left a comment

Choose a reason for hiding this comment

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

LGTM

@Newbie012
Copy link
Contributor Author

Thanks, @hinogi.

What needs to be done in order to merge this PR?

@hinogi
Copy link

hinogi commented Aug 17, 2023

Thanks, @hinogi.

What needs to be done in order to merge this PR?

Guess you'd need a maintainer and 7 successful checks.

@pleerock
Copy link
Member

pleerock commented Jan 2, 2024

Sorry for late answer. This is a good feature to have. We already had an attempt to implement sql template strings here, but PR was not finished. Also, maybe we don't need to introduce new function, and instead re-use exist query function (if that's possible)? We also need documentation and tests for this feature to go.

@gioboa
Copy link
Collaborator

gioboa commented Jan 20, 2025

@Newbie012 thanks for opening this PR.
I'm closing this for now because it's stale, feel free to create a new PR with your improvements. 💪

@Newbie012
Copy link
Contributor Author

Hi @gioboa, I raised a new PR including tests and documentation in #11432

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.

5 participants