Nested queries have limits which can defeat their purpose #41051
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Consider a query
```sql
select count(*) from {{#199}}
```
This query should return the number of distinct rows in the query
defined by 199. But it's actually limited by the excel limit of
1048575. And that's because when the value of `{{#199}}` is expanded it
has that limit applied as normal.
```clojure
qp=> (let [card-id 199] ;; use a valid card id for you
(-> {:database 1,
:type :native,
:native {:query "select count(*) from {{ref}}"
:template-tags {:ref {:card-id card-id
:type :card
:name "ref"
:display-name "ref"}}}
:middleware {:disable-max-results? true}}
qp.compile/compile
:query
(metabase.db.query/format-sql )
println))
select
count(*)
from
(
SELECT
"PUBLIC"."ORDERS"."ID" AS "ID",
"PUBLIC"."ORDERS"."TOTAL" AS "TOTAL"
FROM
"PUBLIC"."ORDERS"
LIMIT
1048575
)
```
But we can suppress this limit when substituting a query inside yielding
```sql
select
count(*)
from
(
SELECT
"PUBLIC"."ORDERS"."ID" AS "ID",
"PUBLIC"."ORDERS"."TOTAL" AS "TOTAL"
FROM
"PUBLIC"."ORDERS"
)
```
And this is proper because we want to limit the _outer_ query, not
internal queries.
|
dpsutton
added
the
backport
|
@dpsutton Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone? |
dpsutton
added a commit
that referenced
this pull request
Apr 10, 2024
* Nested queries have limits which can defeat their purpose
Consider a query
```sql
select count(*) from {{#199}}
```
This query should return the number of distinct rows in the query
defined by 199. But it's actually limited by the excel limit of
1048575. And that's because when the value of `{{#199}}` is expanded it
has that limit applied as normal.
```clojure
qp=> (let [card-id 199] ;; use a valid card id for you
(-> {:database 1,
:type :native,
:native {:query "select count(*) from {{ref}}"
:template-tags {:ref {:card-id card-id
:type :card
:name "ref"
:display-name "ref"}}}
:middleware {:disable-max-results? true}}
qp.compile/compile
:query
(metabase.db.query/format-sql )
println))
select
count(*)
from
(
SELECT
"PUBLIC"."ORDERS"."ID" AS "ID",
"PUBLIC"."ORDERS"."TOTAL" AS "TOTAL"
FROM
"PUBLIC"."ORDERS"
LIMIT
1048575
)
```
But we can suppress this limit when substituting a query inside yielding
```sql
select
count(*)
from
(
SELECT
"PUBLIC"."ORDERS"."ID" AS "ID",
"PUBLIC"."ORDERS"."TOTAL" AS "TOTAL"
FROM
"PUBLIC"."ORDERS"
)
```
And this is proper because we want to limit the _outer_ query, not
internal queries.
* Remove limit from test expectation
* stupid trailing space
* another subquery test
* Use helper function to disable limit middleware
dpsutton
added a commit
that referenced
this pull request
Apr 10, 2024
…41262) * Nested queries have limits which can defeat their purpose Consider a query ```sql select count(*) from {{#199}} ``` This query should return the number of distinct rows in the query defined by 199. But it's actually limited by the excel limit of 1048575. And that's because when the value of `{{#199}}` is expanded it has that limit applied as normal. ```clojure qp=> (let [card-id 199] ;; use a valid card id for you (-> {:database 1, :type :native, :native {:query "select count(*) from {{ref}}" :template-tags {:ref {:card-id card-id :type :card :name "ref" :display-name "ref"}}} :middleware {:disable-max-results? true}} qp.compile/compile :query (metabase.db.query/format-sql ) println)) select count(*) from ( SELECT "PUBLIC"."ORDERS"."ID" AS "ID", "PUBLIC"."ORDERS"."TOTAL" AS "TOTAL" FROM "PUBLIC"."ORDERS" LIMIT 1048575 ) ``` But we can suppress this limit when substituting a query inside yielding ```sql select count(*) from ( SELECT "PUBLIC"."ORDERS"."ID" AS "ID", "PUBLIC"."ORDERS"."TOTAL" AS "TOTAL" FROM "PUBLIC"."ORDERS" ) ``` And this is proper because we want to limit the _outer_ query, not internal queries. * Remove limit from test expectation * stupid trailing space * another subquery test * Use helper function to disable limit middleware Co-authored-by: dan sutton <dan@dpsutton.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #24969
Consider a query
This query should return the number of distinct rows in the query defined by 199. But it's actually limited by the excel limit of 1048575 (meta note, ignore that this is a link. Github is being weird). And that's because when the value of
{{#199}}is expanded it has that limit applied as normal.I thought about trying to propagate that limit middleware, but i think it should not be the solution.
When the parameter substitution code gets to the
{{#199}}parameter tag, it compiles the query for card 199, which naturally includes the default excel limit. But we can suppress this limit specifically as a card parameter value when substituting a query inside yielding