exact_rows_before_limit wrong for numbers/primes/generate_series with LIMIT
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 72/100
Research direction
Run the provided numbers(300) LIMIT 5 SQL reproducer with exact_rows_before_limit = 1, then inspect shouldPushdownLimit in src/Processors/QueryPlan/numbersLikeUtils.cpp and the limit handling in src/Processors/QueryPlan/Optimizations/optimizePrimaryKeyConditionAndLimit.cpp. Compare the behavior with limitPushDown.cpp and a MergeTree query. Done means numbers, numbers_mt, generate_series, and primes report the full pre-LIMIT count while still returning the limited rows.
Written by the indexing model from the issue text.
Description
Describe what's wrong
rows_before_limit_at_leastequals the LIMIT value instead of the true source row count.- Triggered by
SELECT number FROM numbers(300) LIMIT 5with settingexact_rows_before_limit = 1. - Expected
rows_before_limit_at_least: 300, actual returns5. - MergeTree tables report the correct value for the same query shape, so results disagree by source type.
Root cause: shouldPushdownLimit in numbersLikeUtils.cpp has no check for exact_rows_before_limit/alwaysReadTillEnd, so ReadFromSystemNumbersStep caps generation at the LIMIT and the counter never sees the remaining rows; optimizePrimaryKeyConditionAndLimit.cpp has the same missing guard that limitPushDown.cpp already applies.
Analysis details (evidence, affected locations, impact)
Why we believe this is a bug: ReadFromSystemNumbersStep ctor (ReadFromSystemNumbersStep.cpp:431) -> NumbersLikeUtils::getLimitFromQueryInfo (numbersLikeUtils.cpp:133) -> shouldPushdownLimit (numbersLikeUtils.cpp:70-74) returns true -> the step's limit member is set to limit_length + limit_offset -> makePipe shrinks the generated range to it (ReadFromSystemNumbersStep.cpp:660-662) -> the source emits 5 rows, so the counter feeding rows_before_limit_at_least never sees the other 295.
Affected locations:
src/Processors/QueryPlan/numbersLikeUtils.cpp:70—shouldPushdownLimitrejection list -- noexact_rows_before_limit/ always-read-till-end conditionsrc/Processors/QueryPlan/Optimizations/optimizePrimaryKeyConditionAndLimit.cpp:65— second producer of the same source cap;setLimitwithout analwaysReadTillEnd()guard
Impact: With exact_rows_before_limit = 1 -- the setting whose only purpose is to make this statistic exact, documented as 'the data before limit will have to be read completely' (Settings.cpp:3798) -- a LIMIT over numbers/numbers_mt/generate_series/primes returns a rows_before_limit_at_least equal to the LIMIT instead of the true pre-LIMIT count. Clients that use the field for a total-row count (UIs, pagination) get a wrong total with no error. A MergeTree table under the identical query reports the correct count, so the two disagree.
Does it reproduce on most recent release?
Yes — confirmed on current master (commit 827b4f0e149c6).
How to reproduce
Run with SETTINGS exact_rows_before_limit = 1 and compare against the same query over a MergeTree table or a UNION ALL of numbers.
-- Test: exact_rows_before_limit must report the full pre-LIMIT row count for numbers().
SELECT number FROM numbers(300) LIMIT 5
FORMAT JSONCompact
SETTINGS exact_rows_before_limit = 1, output_format_write_statistics = 0;
Note: an automated re-run of this exact block on current master (827b4f0e149c) did not show the failure; the analyst's run did (outputs below). Environment or ordering may matter.
Expected behavior
With exact_rows_before_limit set, rows_before_limit_at_least should reflect the full pre-LIMIT row count, reading all source rows if needed.
Expected output of the reproducer above:
{
"meta":
[
{
"name": "number",
"type": "UInt64"
}
],
"data":
[
[0],
[1],
[2],
[3],
[4]
],
"rows": 5,
"rows_before_limit_at_least": 300
}
Error message and/or stacktrace
The LIMIT is pushed into the source generator, which stops early, so rows_before_limit_at_least equals the LIMIT.
Actual output of the reproducer above on master (827b4f0e149c6):
{
"meta":
[
{
"name": "number",
"type": "UInt64"
}
],
"data":
[
[0],
[1],
[2],
[3],
[4]
],
"rows": 5,
"rows_before_limit_at_least": 5
}
Suggested fix
Add an exact_rows_before_limit rejection to shouldPushdownLimit (it already receives query_info, and its caller getLimitFromQueryInfo holds the ContextPtr), and guard optimizePrimaryKeyConditionAndLimit.cpp:65 with !limit_step->alwaysReadTillEnd() exactly as limitPushDown.cpp:67 does. Trade-off: with the setting on, SELECT ... FROM numbers(N) LIMIT k reads all N rows instead of k -- which is the documented cost of the setting, and what MergeTree already does.
Additional context
Same pattern as #103445 (found by: lexical, vector; vector: cosine distance 0.15 (agrees with the lexical leg)).
Open risks:
always_read_till_endis also set byWITH TOTALS(InterpreterSelectQuery.cpplimitAlwaysReadsTillEnd), but those shapes put an aggregation between the LIMIT and the source, so the walk inoptimizePrimaryKeyConditionAndLimitbreaks beforesetLimitandshouldPushdownLimitrejects onneed_aggregate. I could not construct a WITH TOTALS repro;exact_rows_before_limitis the only trigger I could reach.
Found during automated review of PR #118131; whether that PR introduced it could not be established, so nobody is tagged. Severity P2 · Finding h_pr118131_001
- Dominant language
- C++
- Stars
- 50k
- Forks
- 9k
- Avg merge
- 18h 29m
- Merged PRs (30d)
- 511
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from ClickHouse/ClickHouse
-
comp-datalake
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
ClickHouse/ClickHouse#121222 ·
-
comp-sql-syntax minor
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
ClickHouse/ClickHouse#121170 ·
-
comp-sql-syntax
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
ClickHouse/ClickHouse#121150 ·
-
comp-sql-syntax fuzz
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
ClickHouse/ClickHouse#121027 · 2 comments ·
-
comp-sql-syntax fuzz
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
ClickHouse/ClickHouse#121025 · 2 comments ·
All issues in ClickHouse/ClickHouse
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
-
Sensor initialization takes very long when `--initial-sim-time` is set to current UNIX timestamp Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
gazebosim/gz-sensors#662 · 1 comment ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
LadybirdBrowser/ladybird#12123 ·
-
[adam] AdamNet network read doesn't cap to MAX_ADAM_PACKET_LEN, overflows client receive buffers Open
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
FujiNetWIFI/fujinet-firmware#1649 · 2 comments ·