feat(model): add option to return raw query #11881

seromenho · 2020-01-27T16:36:56Z

Pull Request check-list

Please make sure to review and check all of these items:

Does npm run test or npm run test-DIALECT pass with this change (including linting)?
Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
Have you added new tests to prevent regressions?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
Did you update the typescript typings accordingly (if applicable)?
Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Usage

User.beforeFind(() => {
  console.log('I will be skipped');
});

const sql = await User.findAll({getRawSql: true})
console.log(sql); //  {query: "SELECT ....."}

Implements #2325

I'll come back to this and improve the PR description and add some tests if someone confirms this is a good direction.
Thanks

codecov · 2020-01-27T16:46:39Z

Codecov Report

❗ No coverage uploaded for pull request base (main@51bc05a). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 271c08b differs from pull request most recent head ddb5d73. Consider uploading reports for the commit ddb5d73 to get more accurate results

@@           Coverage Diff           @@
##             main   #11881   +/-   ##
=======================================
  Coverage        ?   96.25%           
=======================================
  Files           ?       94           
  Lines           ?     9214           
  Branches        ?        0           
=======================================
  Hits            ?     8869           
  Misses          ?      345           
  Partials        ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51bc05a...ddb5d73. Read the comment docs.

jdgriffith · 2020-03-06T12:22:48Z

This would be a great addition that our organization would use immediately. Any chance we can get feedback on whether this is a good approach?

tjwhitfield-lilly · 2021-06-03T15:40:04Z

@dleo9307 What is the timeframe on this getting merged / released?

baweinbe · 2021-07-09T04:17:22Z

src/dialects/abstract/query-interface.js

@@ -975,8 +975,13 @@ class QueryInterface {

  async select(model, tableName, optionsArg) {
    const options = { ...optionsArg, type: QueryTypes.SELECT, model };
+    const query = this.QueryGenerator.selectQuery(tableName, options, model);


looks like this should be const query = this.queryGenerator.selectQuery(tableName, options, model);

sdepold · 2021-10-24T18:30:51Z

Hey there :)

First of all: Thanks a bunch for your contribution to Sequelize! Much appreciated!
Second: Unfortunately we haven't had the chance to look into this ever since you created it. Sorry for that!

A couple of months ago, we have switched from master to main branch as our primary development branch and hence this PR is now outdated :(

If you still think this change is making sense, please consider recreating the PR against main. Thanks in advance and sorry for the additional work.

✌️

jdgriffith · 2021-10-24T18:34:42Z

@sdepold Can you comment on whether this is the right approach for retrieving the raw sql? It feels like there has been hardly any discussion from maintainers regarding this PR (maybe I'm missing something). We would love to get this merged into a mainline branch but curious what the likelihood is of that happening.

The base branch was changed.

seromenho · 2021-10-25T11:00:32Z

@sdepold updated the PR and it's now against main branch. 👍

jdgriffith · 2021-11-17T09:48:16Z

Do I dare hope this is about to make it into sequelize?! 😍

sdepold · 2021-11-18T19:00:30Z

if it is WIP: Can we maybe convert it into a draft PR?

sdepold · 2021-11-18T19:02:03Z

fwiw: I just rebased. main is now including some bits and pieces about typescript. it shouldn't make a huge diff

Keimeno

Thanks for your contribution @seromenho, and sorry for the late response!

I've marked two things that I would change about this PR.

There's also a problem inside our lib/model.js, where we make use of the select method.

sequelize/lib/model.js

Lines 1754 to 1770 in 4071378

    
           const results = await this.queryInterface.select(this, this.getTableName(selectOptions), selectOptions); 
        
           if (options.hooks) { 
        
             await this.runHooks('afterFind', results, options); 
        
           } 
        
           //rejectOnEmpty mode 
        
           if (_.isEmpty(results) && options.rejectOnEmpty) { 
        
             if (typeof options.rejectOnEmpty === 'function') { 
        
               throw new options.rejectOnEmpty(); 
        
             } 
        
             if (typeof options.rejectOnEmpty === 'object') { 
        
               throw options.rejectOnEmpty; 
        
             } 
        
             throw new sequelizeErrors.EmptyResultError(); 
        
           } 
        
           return await Model._findSeparate(results, options);

We have a problem here because our result is processed further, we will have to check for these scenarios as well. An early return might be a good solution

Also, I'm unsure if we should limit this option for findAndCountAll, findAll and findOne queries, but it might as well make sense to separate this into multiple PRs

Keimeno · 2021-11-21T14:35:55Z

lib/hooks.js

@@ -90,7 +90,10 @@ const Hooks = {

  async runHooks(hooks, ...hookArgs) {
    if (!hooks) throw new Error('runHooks requires at least 1 argument');
-
+    // Skip hooks if we are only getting raw SQL
+    if (hookArgs[0] && typeof hookArgs[0] === 'object' && hookArgs[0].getRawSql) {


Hooks can also modify the resulting query, such as beforeFind. I think it's best if we modify our lib/model.js to ignore specific hooks.

Keimeno · 2021-11-21T14:39:01Z

lib/dialects/abstract/query-interface.js

@@ -949,11 +949,12 @@ class QueryInterface {

  async select(model, tableName, optionsArg) {
    const options = { ...optionsArg, type: QueryTypes.SELECT, model };
+    const query = this.queryGenerator.selectQuery(tableName, options, model);
+    if (options.getRawSql) {


I'm unsure if getRawSql is the best name for this option, I would probably go with something like queryOnly, getQuery, query or something else that more universally expresses what the expected return type is.

ephys · 2022-04-07T10:24:03Z

One note I have about this PR is that we should avoid having different return types for our functions. It makes typing them and using them with TypeScript much more complicated.

Instead, let's add a new method that does this. Maybe Model.getFindQuery, Model.findAll.getQuery(), or let's keep it as a method on the Query Builder? #394

piotr-pawlowski · 2022-04-07T14:54:51Z

When do you plan to merge this PR? We really need that feature.

ephys · 2022-12-08T17:39:12Z

I'm closing this issue because as indicated here, we're working on making QueryGenerator a public API instead, combined with this other solution and this one

cr0mbly · 2024-04-26T00:47:42Z

I know there was talks about making the QueryGenerator public, but it's been a few years. If this isn't going to be the case we can institute the above PR. Would love to use this natively.

jdgriffith · 2024-04-26T02:16:53Z

A part of me is sad to look back and see my first comment on this thread was four years ago.

I think it's worth merging this PR, assuming it's ready or can be modified easily, as a stop gap until new features or v7 arrives.

Nick-Riggs · 2024-04-26T15:12:55Z

I've needed this for years for different projects, and I've always monkey-patched it. It may not cover every case, but something to the effect of:

    const realQuery = sequelize.query.bind(sequelize);

    sequelize.query = async function (sql: string, options: QueryOptions) {
        if (options?.returnSqlOnly) {
            return sql;
        }

        return realQuery(sql, options);
    }

I hope to see it in the framework proper someday as well.

jdgriffith · 2024-04-26T15:51:42Z

@Nick-Riggs Our team has had to do something similar because we have a dynamic query layer based on the sequelize ORM.

cr0mbly · 2024-05-07T17:47:33Z

@ephys this seems like a pretty big want/need from the community, are we able to look at this again?

I Just note again most ORMs I've dealt with have offered easy exposure of the SQL they're building within the ORM builder itself, it'd be nice if sequlize kept to that convention:

### Rails
Model.all.to_sql
>>> "SELECT `models`.* FROM `models`" 

### Django
Model.objects.all().query()
>>> "SELECT `models`.* FROM `models`"

seromenho changed the title ~~Feat/get raw sql~~ WIP: Feat/get raw sql Jan 27, 2020

seromenho force-pushed the feat/get-raw-sql branch from 4131e9b to 271c08b Compare January 27, 2020 17:16

jy95 mentioned this pull request May 9, 2020

Feature Request: Ability to get raw sql query #2325

Open

seromenho force-pushed the feat/get-raw-sql branch from 271c08b to 057b7b5 Compare August 3, 2020 09:36

dleo9307 approved these changes May 18, 2021

View reviewed changes

baweinbe reviewed Jul 9, 2021

View reviewed changes

dleo9307 previously approved these changes Oct 7, 2021

View reviewed changes

feat(get-raw-sql): get raw sql option

b5fb1f6

seromenho force-pushed the feat/get-raw-sql branch from 057b7b5 to b5fb1f6 Compare October 25, 2021 10:58

seromenho changed the base branch from master to main October 25, 2021 10:58

github-actions bot added the stale label Nov 3, 2021

fzn0x added type: feature For issues and PRs. For new features. Never breaking changes. and removed stale labels Nov 4, 2021

viraldudhat approved these changes Nov 17, 2021

View reviewed changes

Keimeno self-assigned this Nov 17, 2021

Keimeno added the status: awaiting maintainer label Nov 17, 2021

Merge branch 'main' into feat/get-raw-sql

ddb5d73

Keimeno requested changes Nov 21, 2021

View reviewed changes

Keimeno marked this pull request as draft November 21, 2021 14:54

Keimeno changed the title ~~WIP: Feat/get raw sql~~ feat(model): add option to return raw query Nov 21, 2021

Keimeno removed the status: awaiting maintainer label Nov 21, 2021

github-actions bot added the stale label Dec 6, 2021

github-actions bot removed the stale label Apr 8, 2022

ephys closed this Dec 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(model): add option to return raw query #11881

feat(model): add option to return raw query #11881

seromenho commented Jan 27, 2020 •

edited

codecov bot commented Jan 27, 2020 •

edited

jdgriffith commented Mar 6, 2020

tjwhitfield-lilly commented Jun 3, 2021

baweinbe Jul 9, 2021

sdepold commented Oct 24, 2021

jdgriffith commented Oct 24, 2021

seromenho commented Oct 25, 2021 •

edited

jdgriffith commented Nov 17, 2021

sdepold commented Nov 18, 2021

sdepold commented Nov 18, 2021

Keimeno left a comment •

edited

Keimeno Nov 21, 2021 •

edited

Keimeno Nov 21, 2021 •

edited

ephys commented Apr 7, 2022

piotr-pawlowski commented Apr 7, 2022

ephys commented Dec 8, 2022

cr0mbly commented Apr 26, 2024

jdgriffith commented Apr 26, 2024

Nick-Riggs commented Apr 26, 2024

jdgriffith commented Apr 26, 2024

cr0mbly commented May 7, 2024

	const results = await this.queryInterface.select(this, this.getTableName(selectOptions), selectOptions);
	if (options.hooks) {
	await this.runHooks('afterFind', results, options);
	}

	//rejectOnEmpty mode
	if (_.isEmpty(results) && options.rejectOnEmpty) {
	if (typeof options.rejectOnEmpty === 'function') {
	throw new options.rejectOnEmpty();
	}
	if (typeof options.rejectOnEmpty === 'object') {
	throw options.rejectOnEmpty;
	}
	throw new sequelizeErrors.EmptyResultError();
	}

	return await Model._findSeparate(results, options);

feat(model): add option to return raw query #11881

feat(model): add option to return raw query #11881

Conversation

seromenho commented Jan 27, 2020 • edited

Pull Request check-list

Description of change

codecov bot commented Jan 27, 2020 • edited

Codecov Report

jdgriffith commented Mar 6, 2020

tjwhitfield-lilly commented Jun 3, 2021

baweinbe Jul 9, 2021

Choose a reason for hiding this comment

sdepold commented Oct 24, 2021

jdgriffith commented Oct 24, 2021

seromenho commented Oct 25, 2021 • edited

jdgriffith commented Nov 17, 2021

sdepold commented Nov 18, 2021

sdepold commented Nov 18, 2021

Keimeno left a comment • edited

Choose a reason for hiding this comment

Keimeno Nov 21, 2021 • edited

Choose a reason for hiding this comment

Keimeno Nov 21, 2021 • edited

Choose a reason for hiding this comment

ephys commented Apr 7, 2022

piotr-pawlowski commented Apr 7, 2022

ephys commented Dec 8, 2022

cr0mbly commented Apr 26, 2024

jdgriffith commented Apr 26, 2024

Nick-Riggs commented Apr 26, 2024

jdgriffith commented Apr 26, 2024

cr0mbly commented May 7, 2024

seromenho commented Jan 27, 2020 •

edited

codecov bot commented Jan 27, 2020 •

edited

seromenho commented Oct 25, 2021 •

edited

Keimeno left a comment •

edited

Keimeno Nov 21, 2021 •

edited

Keimeno Nov 21, 2021 •

edited