mirror of
https://github.com/duckdb/duckdb.git
synced 2026-10-28 08:46:33 +08:00
main
90540
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Max Gabrielsson
|
ad2d8e9acc |
Unify function overload selection and improve function binding errors (#27703)
# Motivation
Binding functions has two steps:
1. Select an overload from a `FunctionSet<T>` given some argument types,
get back a `optional_idx` referring to the best matching overload (or
none if none matched).
2. Extract the "definition" (`shared_ptr<const SimpleFunction>`) from
the set, and pass it to the binder with a set of argument _expressions_,
and get back a `unique_ptr<BoundScalarExpression>` (or the equivalent
for other function types)
Over time the`FunctionBinder` has grown a lot in complexity. We now have
to deal with template types, named arguments, optional arguments with
default values, `*args` and `**kwargs`, which makes the both the
overload __selection__ (which has to check/reorder argument types) and
the __binding__ (which has to do the same for expressions) duplicate a
lot of work.
In this PR we now front-load and cache a lot of this work in the
overload-selection stage. Instead of just returning a `optional_idx`, we
now return a `FunctionBinding`, which holds the definition of the
overload, the actual resolved types (after templated resolution, `ANY`
types are gone, default-value insertion), and the mapping from input
argument to signature parameter - which we now can re-use when we
actually bind the inputs to produce the final bound function expression.
Additionally, we also save information about the overloads which
_didn't_ match, which allows us to produce much more detailed error
messages, instead of just dumping all the other overloads in the set as
"candidates" in the error message.
# What Changes
## General cleanup in the `FunctionBinder`
Lots of templating and redundant overloads have been removed. Its also
much easier/robust to do things like `set.GetFunctionByArguments()` with
the new `FunctionCallArguments` helper struct to select/bind functions
internally in e.g. the optimizer.
## Template inference as part of overload selection
Template parameter deduction (for functions that have
`LogicalType::TEMPLATE` as parameter/return types) is now part of
overload resolution, instead of being a hard error enforced at
expression-binding time (after the overload has already been selected).
This makes it possible to create more advanced functions with multiple
template based overloads. E.g. if a templated overload doesn't match, we
still have the chance to pick another overload that may, or if there are
multiple template overloads with different costs, we'll pick the
cheapest.
## Better "candidate list" error messages when overload selection fails
This is the main user facing change is that "candidate list" in error
messages are more informative.
We now print the "reason" an overload was _not_ selected, as well as
group candidates by reason. This is the main reason why the diff in this
PR is so large - lots of formatting and polymorphic error juggling code.
I know the below examples maybe aren't the prettiest - Im definitely
planning on reworking how we format/display error messages in the future
- but the main point is that this information is now _there_. We
actually store the `QueryLocation` for where exactly an argument was
passed wrong, for each failed overload, which is pretty precise
information.
I've also attempted to keep the first line `Binder Error: No function
...` the same to not have to change _too_ many test files right now
(saving that for the future formatting change)
Here are some examples:
### Missing parameters are grouped across overloads
```
SELECT substring('abc');
----
Binder Error: No function matches the given name and argument types 'substring(STRING_LITERAL)'. You might need to add explicit type casts.
Candidates:
'"substring"(string VARCHAR, "start" BIGINT) -> VARCHAR'
'"substring"(string VARCHAR, "start" BIGINT, length BIGINT) -> VARCHAR'
Required parameter "start" is missing
```
### A typo in a named argument is reported as the reason
```
SELECT round(1.5, foo := 2);
----
Binder Error: No function matches the given name and argument types 'round(DECIMAL(2,1), "foo" := INTEGER_LITERAL)'. You might need to add explicit type casts.
Candidates:
'round(x TINYINT) -> TINYINT'
'round(x TINYINT, "precision" INTEGER) -> TINYINT'
'round(x SMALLINT) -> SMALLINT'
... and 13 more
Has no parameter named "foo"
```
### Template deduction failures show how the type was inferred
```
SELECT [1, DATE '2020-01-01'];
----
Binder Error: No function matches the given name and argument types 'list_value(INTEGER_LITERAL, DATE)'. You might need to add explicit type casts.
Candidates:
'list_value("value" T, /, *args T) -> T[]'
Cannot deduce template type 'T', it was inferred to be:
- 'INTEGER_LITERAL', from first occurrence
- 'DATE', which is incompatible with the previously inferred type
'list_value() -> "NULL"[]'
Takes no positional arguments
```
### Each overload gets its own reason, closest first
```
SELECT * FROM range(1, 10, 2, 4);
----
Binder Error: No function matches the given name and argument types 'range(INTEGER_LITERAL, INTEGER_LITERAL, INTEGER_LITERAL, INTEGER_LITERAL)'. You might need to add explicit type casts.
Candidates:
'"range"("start" BIGINT, stop BIGINT, step BIGINT, /)'
'"range"("start" TIMESTAMP, stop TIMESTAMP, step INTERVAL, /)'
'"range"("start" TIMESTAMP WITH TIME ZONE, stop TIMESTAMP WITH TIME ZONE, step INTERVAL, /)'
Takes at most 3 positional arguments
'"range"("start" BIGINT, stop BIGINT, /)'
Takes at most 2 positional arguments
'"range"(stop BIGINT, /)'
Takes at most 1 positional argument
```
### Type constructors use the same machinery
This is one of the really cool parts IMO. Since type constructors now
have `FunctionSignature`s, they participate in overload resolution the
same way as functions do.
```
SELECT NULL::DECIMAL('ten');
----
Binder Error: Type "DECIMAL" does not accept type parameters (VARCHAR)
Candidates:
'DECIMAL(width UTINYINT, scale UTINYINT := 0)'
Argument 1 of type VARCHAR cannot be implicitly cast to UTINYINT
'DECIMAL()'
Takes no positional arguments
```
```
SELECT NULL::MAP;
----
Binder Error: Type "MAP" requires type parameters
Candidates:
'MAP(key TYPE, value TYPE)'
Required parameter "key" is missing
```
### Ambiguous calls list the tied candidates
```
SELECT date_part(NULL, NULL);
----
Binder Error: Could not choose a best candidate function for the function call "date_part("NULL", "NULL")". In order to select one, please add explicit type casts.
Candidates:
'date_part(part VARCHAR, ts DATE) -> DOUBLE'
'date_part(part VARCHAR, ts INTERVAL) -> DOUBLE'
'date_part(part VARCHAR, ts TIME) -> DOUBLE'
... and 2 more
All match the given arguments equally well
```
# Future work
The main reason I'm excited for this is that resolving types (and
templates) at overload-selection time unblocks work on a lot of cool
stuff. One of these things are typed lambda functions (like being able
to define `LogicalType::LAMBDA(INT, INT) -> INT`), but the main thing
is: _removing_ `LogicalTypes` from the `FunctionSignature`.
This would basically turn all parameters in `FunctionSignatures` into
"unbound types", which would be resolved at bind/overload selection time
into concrete logical types - kinda like how `MacroFunction`s work.
Having that work would allow defining functions with non-type template,
e.g. `array_concat<N, M>(a INT[N], b INT[M]) -> INT[N + M)`, as well as
move `MacroFunction`'s to also use the `FunctionSignature` enabling fun
stuff like variadic macros (taking `*args`/`**kwargs` and/or lambda
functions).
|
||
|
Mark
|
e78b080cc7 |
Fix list_has_any collations, fmod, week parts, string_split, DECIMAL casts and double formatting (#27762)
Fixes wrong results and errors in scalar functions and casts: `list_has_any`/`list_has_all` under collations, `fmod` with large quotients, week-based date parts in the first year, splitting an empty string by an empty delimiter, casting `true` to a DECIMAL whose width equals its scale, and double formatting that depended on the C++ standard library. Combines the PRs labeled `Ready To Merge` in https://github.com/duckdb-bot/duckdb/pulls below, on top of the current head of `main`. The changes of each PR are applied as one squashed commit. ## Changes applied (6 PRs) - duckdb-bot/duckdb#217: list_has_any and list_has_all compare elements under their collation - duckdb-bot/duckdb#8: Compute fmod from the exact remainder so large quotients keep it - duckdb-bot/duckdb#60: Compute week-based date parts of dates in the first year - duckdb-bot/duckdb#61: Split an empty string by an empty delimiter into no characters - duckdb-bot/duckdb#181: Casting true to a DECIMAL whose width equals its scale fails - duckdb-bot/duckdb#10: Format doubles in EXPLAIN, JSON maps and range errors the same with every C++ standard library ## Original descriptions <details><summary>duckdb-bot/duckdb#217: list_has_any and list_has_all compare elements under their collation</summary> `list_has_any` and `list_has_all` returned false for `['A' COLLATE NOCASE]` and `['a']`, even though `list_contains` matched them. Filters could therefore drop matching rows. The same problem affected their aliases, operators, and comparisons involving collated or VARIANT fields inside STRUCTs. The functions compared sort keys of stored values without asking the binder to apply collations first. `list_contains` and `list_position` already push collations into their arguments, turning values into collation keys before comparing them. This two-line change gives both functions the same collation handling. Top-level VARIANT matching in duckdblabs/bot-issue-tracker#659 awaits the `PushCollations` fix in duckdblabs/bot-issue-tracker#658. `list_intersect` still needs a way to compare keys while returning original elements; `default_collation` remains unchanged. The new regression test, `test/sql/collate/test_collate_list_has_any_all.test`, fails on main and passes with the change. Targeted list, collation, and variant tests pass in reldebug and in relassert with sanitizers. The full reldebug suite passed 8885 tests; relassert without slow tests passed 8042. The only failure in both was `test/sql/settings/reset/reset_threads.test`, which also fails on main. Related: duckdblabs/bot-issue-tracker#659 </details> <details><summary>duckdb-bot/duckdb#8: Compute fmod from the exact remainder so large quotients keep it</summary> Moved from duckdb/duckdb#27315. `fmod(1e16, 3.0)` and `fmod(1e300, 7.0)` returned `0.0` instead of `1.0`, including in v1.5.6. `1e16::DOUBLE % 3.0` already returned the correct result. Calls such as `fmod(nextval('s'), 4)` also advanced the sequence twice per row and returned wrong values. The SQL macro computed `x-y*floor(x/y)`. With large quotients, floating-point rounding could make the product equal to `x`, losing the remainder during subtraction. Expanding the macro also repeated its arguments, so expressions with side effects were evaluated more than once. This replaces the macro with a scalar function using `ModuloOperator` (`std::fmod`) and a divisor-sign adjustment, preserving return types, positive zero and zero-divisor settings. Results reflect the stored floating-point values: `fmod(6.0, 0.2)` now returns `0.19999999999999968` instead of `0.0`, because `DOUBLE` `0.2` is slightly larger than 0.2. Infinite divisors now follow floored-remainder semantics. `test_27102.test` covers numerical edge cases, types, `NULL`s, single argument evaluation, named arguments and zero-divisor settings; `test_fdiv_fmod.test` passes unchanged. The reporter's duckdb/duckdb#27106 rewrites the macro using `mod(x, y)`, but integer arithmetic makes `fmod(5, 0)` error instead of returning `NaN` under default settings. Fixes duckdblabs/bot-issue-tracker#10 Related: duckdb/duckdb#27102 </details> <details><summary>duckdb-bot/duckdb#60: Compute week-based date parts of dates in the first year</summary> For the smallest date, `DATE '5877642-06-25 (BC)'`, extracting `weekofyear`, `isoyear`, `yearweek` or `week` raises `Conversion Error: Date out of range: -5877641-1-1`, even though these functions return integers. Formatting with `strftime` using `%U`, `%W` or `%V` fails the same way. Both ISO and regular week calculations build January 1 of the input year using `Date::FromDate`. For the minimum year, January 1 falls before the smallest supported date, outside the 32-bit day range. The input is valid, but this intermediate date is not. We now use 64-bit day numbers for January 1 and the week calculations, deriving January 1 of the minimum year from 400 years later using the calendar’s repeating cycle. Regression tests fail on main and cover year 359’s matching calendar, ISO week 53 across the year boundary, the maximum date, and lists of date parts. The JSON COPY test now checks the formatted ISO year because `%G` no longer raises an error. thatssoheil’s duckdb/duckdb#25143 took a similar approach but was closed as stale without being merged. Fixes duckdblabs/bot-issue-tracker#169 Related: duckdb/duckdb#25110 </details> <details><summary>duckdb-bot/duckdb#61: Split an empty string by an empty delimiter into no characters</summary> `str_split('foo', '')` returns one element per character, but `str_split('', '')` returns `['']` even though there are no characters to split. Joining an empty list with `''` and splitting it again therefore does not recover the original list. `string_split` and `regexp_split_to_array` behave the same with an empty delimiter or pattern. This happens because `StringSplitter::Split` always emits the remainder after the last match, including when the input is empty. That adds one empty element to the result. The change makes an empty string split into `[]` with an empty delimiter. Non-empty delimiters still produce `['']`, matching Python's `''.split(',')`. The regression test in `test/sql/function/string/test_split_empty_string.test` fails on main and covers all three functions with empty and non-empty delimiters, `NULL`, and `regexp_split_to_table`. Fixes duckdblabs/bot-issue-tracker#304 Related: duckdb/duckdb#14414 </details> <details><summary>duckdb-bot/duckdb#181: Casting true to a DECIMAL whose width equals its scale fails</summary> Casting `true` to a decimal with equal width and scale returned a fraction instead of failing. For example, `true::DECIMAL(2,2)` gave `.01`, although casting integer `1` already failed. `TRY_CAST` returned `.01` instead of `NULL`, and inserts stored it. All widths from 1 to 38 were affected. `TryCastBoolToDecimal` scaled `true` only when width exceeded scale. Otherwise, it used an integer cast and stored an unscaled `1`, representing `10^-scale`. The existing test, added together with this code in 2021, checked that the result cast back to `true` without checking its value. The change rejects `true` through normal cast error handling, so `TRY_CAST` returns `NULL`; `false` still gives zero. `test/sql/cast/boolean_to_decimal_cast.test` covers width boundaries, batches and inserts, failing on main and passing here. `test/sql/types/numeric/bool_casts.test` now checks errors and `TRY_CAST` results. Cast, decimal, variant and optimizer statistics tests pass in reldebug and relassert with sanitizers. The full reldebug suite had 8777 passes; relassert without slow tests had 7943. The only failures were the existing boolean test, corrected in the second commit and now passing on both builds, and `reset_threads.test`, which also fails on main. Set operations selecting these decimals now fail too; their type choice is tracked separately in duckdblabs/bot-issue-tracker#622. Fixes duckdblabs/bot-issue-tracker#617 </details> <details><summary>duckdb-bot/duckdb#10: Format doubles in EXPLAIN, JSON maps and range errors the same with every C++ standard library</summary> Moved from duckdb/duckdb#27317. With C++26 and libstdc++ 16, `EXPLAIN (FORMAT JSON) SELECT * FROM integers LIMIT 10%` prints `"Limit": "10%"` instead of `"Limit": "10.000000%"`, causing `test_explain_limit.test` to fail. Sample percentages and some error messages also change, making their text depend on the standard library used to build DuckDB. This happens because `duckdb::to_string` is `std::to_string`. C++26’s P2587R3 changes double formatting from `%f` to the shortest round-trip form. DuckDB already accepts C++26 builds, and libc++ and the MSVC STL will produce the same changes when they implement it. We replace the six floating-point calls with `StringUtil::Format("%f", ...)`, preserving C++17 output for limit and sample percentages, JSON map doubles and double out-of-range errors. A build with temporarily deleted floating-point overloads found no other calls across `src/`, in-tree extensions and tests; those overloads are excluded to preserve extension compatibility. The existing EXPLAIN, sample and limit tests and the full unit suite pass. Our GCC 13 and Clang 18 toolchains do not implement P2587R3, so no test fails before the change locally. Fixes duckdblabs/bot-issue-tracker#39 Related: duckdb/duckdb#26611 </details> |
||
|
Mark
|
e270f5740e |
Fix encrypted Parquet DataPageV2, interval bloom filters and union_by_name BLOB (#27766)
Fixes Parquet and multi-file reads: encrypted DataPageV2 pages written by Arrow, hashing of normalized intervals in bloom filters, and VARCHAR columns that union_by_name turns into BLOB. Combines the PRs labeled `Ready To Merge` in https://github.com/duckdb-bot/duckdb/pulls below, on top of the current head of `main`. The changes of each PR are applied as one squashed commit. ## Changes applied (3 PRs) - duckdb-bot/duckdb#205: Read encrypted Parquet DataPageV2 pages written by Arrow - duckdb-bot/duckdb#180: Hash normalized Parquet intervals with their whole days in the days field, and no hash for negative ones - duckdb-bot/duckdb#31: Read a VARCHAR file column as its UTF-8 bytes when union_by_name makes it BLOB ## Original descriptions <details><summary>duckdb-bot/duckdb#205: Read encrypted Parquet DataPageV2 pages written by Arrow</summary> DuckDB could not read Parquet files with modular encryption and `DataPageV2` pages written by parquet-rs, as reported in duckdb/duckdb#24785. Even with the correct footer key, an uncompressed page failed with “uncompressed page size mismatch (expected 4, actual: 36)” and a Snappy page with “Too many bytes requested from crypto buffer”. A page with compressed values failed with “Computed AES tag differs from read AES tag”. DuckDB 1.5.6 also fails. Arrow writers include 32 bytes of encryption framing (length, nonce and tag) in `compressed_page_size`. `ColumnReader::PreparePage` already subtracts these for `DataPage` and dictionary pages in files with a file AAD, but `PreparePageV2` used the size as written. It also made separate decrypting reads for the uncompressed level prefix and compressed values, although the whole page body is one encrypted module. DuckDB-written files are unaffected because DuckDB writes `DataPage` (V1) headers even with `PARQUET_VERSION V2`. This change shares the size adjustment through `ColumnReader::GetPageBodySize`, which throws a `DataCorruptionException` for sizes below the framing instead of wrapping around. `PreparePageV2` checks against the adjusted size, reads the body once, copies the level prefix and decompresses the values when needed. Unencrypted V2 pages use the same path, adding one copy of the level bytes. The reporter’s open pull request, duckdb/duckdb#24406, makes the same correction; this change reuses its four fixtures, whose pages parquet-rs stores uncompressed because they are small. Two additional fixtures generated with parquet-rs 58.4.0, writer version 2.0, Snappy, dictionaries disabled and 4,096 rows each, one with `NULL`s, cover compressed values with a level prefix. `test/sql/copy/parquet/encryption/arrow_compatibility.test` checks values, `NULL` positions and counts across all six fixtures. The new cases fail on `main`; subtracting the framing while retaining two reads still fails the compressed cases. The size guard was verified for `DataPage`, dictionary and `DataPageV2` pages by setting a page size to 31 in a debugger. It has no automated test because that would require encrypting a page header by hand. `test/sql/copy/parquet`, `test/parquet` and `[parquet]` pass in reldebug and sanitizer builds. The full unit suite passes apart from failures also seen on `main` (`reset_threads.test` under the runner’s `--max-threads`, `external_resource_logging.test`, `external_resource_lifecycle.test` and httpfs’s `http_logging.test`) and `connect_passthrough_quack.test`, which fails only alongside the other connect tests. Quack’s tests hung once on their fixed ports and pass when run in parts. Fixes duckdblabs/bot-issue-tracker#190 Related: duckdb/duckdb#24785 </details> <details><summary>duckdb-bot/duckdb#180: Hash normalized Parquet intervals with their whole days in the days field, and no hash for negative ones</summary> At the 32 bit month limit, `SELECT count(*) FROM 'iv.parquet' WHERE iv = INTERVAL '-178956970 years -8 months -1 day'` failed with `INTERNAL Error: Information loss on integer cast: value -1 outside of target range [0, 4294967295]` and invalidated the database instead of returning 0. For intervals whose normalization saturates both months and days, such as `to_months(2147483646) + to_days(2147483647) + INTERVAL '744 hours'`, main's probe hash did not match the bloom filter written by v2.0 (`v2.0-cyanoptera`): `parquet_bloom_probe` returned true, skipping the row group containing the value. Both versions mark these files `duckdb.interval_bloom_filter = normalized-v1`. Only intervals of about 178956970 years or more are affected; v1.5.6 has no interval bloom filters. DuckDB adds a second bloom filter hash for an INTERVAL's normalized value. `ParquetIntervalUtils::HashNormalized` in `extension/parquet/include/parquet_interval.hpp` used `Encode`, the Parquet encoding for stored values. When normalization saturates, `interval_t::Normalize` can leave negative days, making `Encode`'s checked `NumericCast<uint32_t>` throw. It can also leave whole days in microseconds. Commit `19cf7eccf72` made `Encode` keep microseconds in the milliseconds field when they fit. This is correct for stored values, but sharing the function also changed their normalized hash: v2.0 and earlier main builds put those whole days in the days field. `ParquetIntervalUtils::TryHashNormalized` now encodes the normalized interval itself, putting whole days in the days field as v2.0 does. It returns no hash if any part is negative, since Parquet files hold no negative intervals: the writer rejects them. Hashes remain unchanged across all versions when normalization does not saturate. The writer keeps the v2.0 form for saturated intervals so v2.0 finds them in new files. The reader bypasses bloom filters when a constant's normalization leaves whole days in microseconds, because main files written since `19cf7eccf72` hash those in milliseconds while v2.0 files hash them in days. This retains row groups from both and evaluates the filter on their rows. `test/sql/copy/parquet/bloom_filter_interval_saturated.test` fails on main and covers the negative constant, including `IN`, the saturated interval in a new file, and `parquet_bloom_probe` on `data/parquet-testing/interval_bloom_saturated_v2_0.parquet` (written by `v2.0-cyanoptera` at `d54cbbe3df1`) and `data/parquet-testing/interval_bloom_saturated_main.parquet` (written by main at `5d10bf46159`). The v2.0 file stores that value with a days field above the 32 bit range, which decodes wrongly in every version, so its test only probes the bloom filter; `19cf7eccf72` (duckdb/duckdb#27471) fixed writing the Parquet interval time part. All unit tests pass on reldebug except failures also present on `main` on this machine. Parquet copy tests pass on relassert with sanitizers. No benchmark was run: the writer computes the same normalized hash per value as before. These failures were found while merging main into duckdb-bot/duckdb#129; this change also relates to duckdblabs/bot-issue-tracker#207. Fixes duckdblabs/bot-issue-tracker#616 </details> <details><summary>duckdb-bot/duckdb#31: Read a VARCHAR file column as its UTF-8 bytes when union_by_name makes it BLOB</summary> Moved from duckdb/duckdb#27348. Reading Parquet files with `union_by_name = true` can fail when column `p` is `VARCHAR` in one file and `BLOB` in another. Text such as `café` raises a conversion error, while ASCII text succeeds. A literal `\x41` silently becomes the byte `A`. This happens because union by name selects `BLOB` as the common type, and the multi-file column mapper applies the SQL cast from `VARCHAR` to `BLOB`. That cast parses blob literals with `\x` escapes, which is appropriate for SQL literals but changes the meaning of text stored in a file. `ConstructMapExpression` now uses a reinterpret cast for `VARCHAR` file columns returned as `BLOB`, preserving their UTF-8 bytes as `encode()` does. This also covers Parquet `COPY ... FROM` into BLOB columns. CSV keeps its own escape parsing; lists and structs are tracked separately. The regression test in `union_by_name_varchar_blob.test` fails on main and checks non-ASCII text, literal `\x41`, NULLs and empty values in both file orders, including byte values, filtering and `COPY ... FROM`. Fixes duckdblabs/bot-issue-tracker#110 Related: duckdb/duckdb#26040 </details> |
||
|
Mark
|
d895f35380 |
Render, cast and split every finite timestamp, also before the first midnight of the range (#27763)
Fixes rendering, casting and splitting of finite timestamps whose day starts below the timestamp range. Combines the PRs labeled `Ready To Merge` in https://github.com/duckdb-bot/duckdb/pulls below, on top of the current head of `main`. The changes of each PR are applied as one squashed commit. ## Changes applied (1 PR) - duckdb-bot/duckdb#25: Render, cast and split every finite timestamp, also before the first midnight of the range ## Original descriptions <details><summary>duckdb-bot/duckdb#25: Render, cast and split every finite timestamp, also before the first midnight of the range</summary> Moved from duckdb/duckdb#27338. DuckDB could store a finite `TIMESTAMP` before `290309-12-22 (BC) 00:00:00`, such as `make_timestamp(-9223372036854775806)`, and both `isfinite` and `duckdb_is_finite_timestamp` returned `true`. But casting it to `VARCHAR` raised `INTERNAL Error: Scalar function "__cast" threw an execution error, but the function is not marked as fallible`, while `duckdb_from_timestamp` threw through the C API and ended the process, as reported in duckdb/duckdb#26797. `epoch_ms`, `make_timestamp_ms`, `to_timestamp` and `time_bucket` also raised range errors, as did interval arithmetic and string casts landing on the first partial day. This still happens on main at duckdb/duckdb@8fddcad6ea: duckdb/duckdb#27627 combined duckdb-bot/duckdb#156, duckdb-bot/duckdb#168, duckdb-bot/duckdb#169 and duckdb-bot/duckdb#189, but did not include this pull request. The problem came from splitting timestamps into dates and times. When `test_all_types` was added, |
||
|
Maxxen
|
0bef2c0959 | Merge branch 'main' of github.com:duckdb/duckdb into simplify-function-binding | ||
|
Mark
|
cd2832d25f |
JSON: read a DECIMAL column from the number's text, not through a double (#27577)
`read_json` parses every number as a double before converting it to the column's type, so a `DECIMAL(38,18)` column loses digits silently and a number with more than 20 integer digits fails with the double's rendering rather than the number in the file. #25006 fixed the same bug for the cast and `json_transform` paths by parsing with `YYJSON_READ_NUMBER_AS_RAW` when the target contains a DECIMAL; the reader kept the double path. ```sql -- {"d": 12345678901234.567890123456789012} SELECT d FROM read_json('f.json', columns = {d: 'DECIMAL(38,18)'}); -- before: 12345678901234.569054409354903552 -- after: 12345678901234.567890123456789012 ``` The reader now does the same. `JSONReader` computes its read flags once from the column types and keeps numbers as text when a column is or contains a DECIMAL; the DECIMAL column is cast from the text. This also changes how a narrow DECIMAL rounds a tie: `1.005` into `DECIMAL(4,2)` is now `1.01`, as `CAST('1.005' AS DECIMAL(4,2))` gives, rather than `1.00` from the nearest double. The other columns of such a document must read as they do without the DECIMAL column: - `JSONCommon::ReadRawNumber` parses the text back with `yyjson_read_number` under the default flags, so an integer that fits an (u)int64 or a finite double converts as before and what those flags keep as text (`YYJSON_READ_BIGNUM_AS_RAW`) stays a string cast. `GetValueNumerical`, `GetValueString` and the GeoJSON coordinate reader use it. - A nested value that becomes a VARCHAR, BLOB or JSON column goes through `JSONRawNumberWriter`: a value without numbers is written as is, otherwise a mutable copy is written with its numbers parsed, so `[1.50]` reads as `[1.5]` either way. `JSONTransformOptions::numbers_as_raw` carries this per scan, decided at bind like `date_format_map`. `json_transform` with a DECIMAL target had the same gap since #25006 and gets the same treatment. One visible consequence: it used to cast the other numbers of such a document from their text, so `9007199254740993.0` into BIGINT was exact only when a DECIMAL sibling was present; it now converts through a double as it does without one, and a DECIMAL target is what keeps the digits. Files without a DECIMAL column are parsed as before. **Testing:** `test/sql/json/table/read_json_decimal.test` (146 assertions): 32- and 38-digit numbers through `read_json`, `COPY ... FROM`, `format = 'array'`, `records = false` and a bare number array; DECIMAL inside STRUCT, LIST, MAP and UNION; narrow DECIMAL columns and the tie case; quoted numbers and NULL; a value too wide for the type (error naming the value, or NULL with `ignore_errors`), `Infinity` and `1e400` (error naming the value); BOOLEAN, BIGINT, UBIGINT, HUGEINT, FLOAT, INTEGER, VARCHAR, BLOB, JSON and GEOMETRY columns next to a DECIMAL column, each asserted equal to the result without it, including nested values and a JSON column without numbers; a GeoJSON Feature with a DECIMAL property; `json_transform` with a DECIMAL target, including an integer beyond 2^53 written with a fraction next to it; two files with and without `maximum_sample_files = 1`; `1e400` and `Infinity` into DOUBLE and BOOLEAN; auto-detection and `read_json_objects` unchanged. `test/sql/json/*` passes under relassert with ASan and UBSan. |
||
|
Mark
|
3dc6a1a971 |
LPAD and RPAD: copy the padding from a per-call plan instead of decoding it character by character (#27858)
Fixes #27846. `InsertPadding` decoded one pad character per output character, flushing the pad whenever it ran out, so a 200-character pad cost 200 decodes and as many one-byte inserts per row. Since the decoder became an out-of-line call (#26485) and the result size check counted the pad again on every row (#26482), `lpad(x, 200, '0')` takes 1.75x as long on v2.0.0 as on v1.5.5 in the report, and about ten times as long as `repeat('0', 200) || x` on both. **The change** - The pad's geometry is counted once per call: its characters, the whole copies the padding needs, and the bytes of the remainder. The size check uses the same plan instead of counting again, the buffer is reserved to the result size, and the padding is copied whole, a single byte at a time for a one-byte pad. LEFT and RIGHT share one function. - Results and errors are unchanged: the existing tests pass, including the multibyte remainders and the maximum-size errors. **Measured** on an M-series Mac, one thread, 2M rows, seconds, best of 3: | statement | v1.5.5 | v2.0.0 | main | this PR | |---|---:|---:|---:|---:| | `lpad(i::VARCHAR, 18, '0')` | 0.131 | 0.151 | 0.145 | 0.098 | | `lpad(i::VARCHAR, 50, '0')` | 0.277 | 0.347 | 0.328 | 0.115 | | `lpad(i::VARCHAR, 200, '0')` | 0.984 | 1.227 | 1.114 | 0.126 | | `rpad(i::VARCHAR, 200, '0')` | 0.966 | 1.212 | 1.112 | 0.129 | | `lpad(i::VARCHAR, 200, 'ab😀')` | 0.979 | 1.313 | 1.264 | 0.628 | | `repeat('0', 200) \|\| i::VARCHAR` | 0.102 | 0.085 | 0.081 | 0.082 | **Verification** - `test/sql/function/string/test_pad.test` gains one-byte, one-character multibyte and longer-than-needed pads, a multibyte pad counted in characters, and the empty pad with and without padding needed; the string function group passes. - `benchmark/micro/string/lpad.benchmark`, registered for the regression job, tracks the one-byte case. |
||
|
Mark
|
a494af27aa |
Add A Pre-Bind Hook To PlannerExtension (#27872)
Binding looks up tables one at a time. For a remote catalog, such as an Iceberg REST catalog, that means a query waits on one round trip per table the query reads, and the catalog can't overlap them because it only ever sees the current lookup. This PR adds a pre-bind hook to `PlannerExtension`, next to the existing post-bind one. The planner calls it right before binding, with the statement: ```cpp typedef void (*pre_bind_function_t)(PlannerExtensionInput &input, SQLStatement &statement); ``` An extension can use it to look at the statement and start loading the catalog entries it will need. duckdb/duckdb-iceberg#1646 uses it to send the `LoadTable` requests of a statement's tables together. |
||
|
Mark
|
bcd6984cd0 |
Reduce extension CI workload by ignoring changes to /extension/** (#27957)
Originally, when changing files in `extension/**`, we would: - Disable reduced-CI mode. - Add the macOS job. - Add a Linux ARM64 release build alongside AMD64. - Expand extension builds to Windows ARM64, Linux ARM64 musl, and WASM EH. This PR only reduces the extension job matrix and prerequisites. Code in `extension/**` is still built on a smaller number of platforms. |
||
|
Mark
|
d5e4216981 |
StatisticsPropagator: Convert COMPARE_(NOT_)DISTINCT_FROM to COMPARE_(NOT)EQUAL (#27945)
Was surprised we didn't do this yet. Skipped `MARK` joins for now to keep it simple. |
||
|
Louie Duck
|
cfd1c0d89a |
Merge remote-tracking branch 'upstream/main' into pad-without-per-character-decode
# Conflicts: # .github/regression/micro.csv |
||
|
Robinho-MR
|
b74a7d5a74 | Merge remote-tracking branch 'origin/main' into scalar-function-fixes | ||
|
Robinho-MR
|
34367524e9 | Merge remote-tracking branch 'origin/main' into timestamp-rendering-fixes | ||
|
Robinho-MR
|
ebc5997c21 | Merge remote-tracking branch 'origin/main' into parquet-file-fixes | ||
|
Laurens Kuiper
|
0ccee9a087 |
Preserve constant and dictionary vectors in ColumnDataCollection (#27380)
ColumnDataCollection flattens every appended vector, so a dictionary-compressed string column or a constant column loses its compact form at the first materialization point and every downstream consumer pays flat-copy costs in time and memory. Following the review discussion, this PR replaces the earlier shared-list mechanism with a general representation contract: the collection preserves CONSTANT and DICTIONARY vectors end to end, and the shared-list idea returns later as another class of the same contract. ### Mechanism - Each vector entry carries a representation tag (FLAT, CONSTANT, DICTIONARY) on `VectorMetaData`. Recognition happens once at the top-level `Append` dispatch. The per-type copy functions are untouched and remain the universal fallback. - CONSTANT stores the value plus a row count. Fixed-size values and inlined strings live in the entry metadata, longer string bytes get one heap entry. An append with the same value extends the entry, a mismatch materializes it in place and continues flat. - DICTIONARY stores `sel_t` codes per entry and the dictionary values once per segment, deduplicated by dictionary id. Eligibility: a non-empty dictionary id (storage scans, the parquet reader, and the expression cache mint them), dictionary size at most `STANDARD_VECTOR_SIZE`, a flat child, value width at least `sizeof(sel_t)`, and for buffer-managed VARCHAR a dictionary heap of at most one block. - Scans hand out real CONSTANT or DICTIONARY vectors under `ALLOW_ZERO_COPY`, with codes and string bytes referenced zero-copy from pinned blocks and the dictionary id preserved, so re-appending a scanned chunk re-recognizes. `DISALLOW_ZERO_COPY` materializes flat exactly as before, and client result fetches stay on that path. - All compact data lives in ordinary buffer-managed blocks, so spilling needs no special hook. Eviction plus the existing swizzle repair cover dictionary strings, and constants reconstruct their string pointer from the pinned block. - Top-level entry storage is allocated on first write instead of per chunk up front. This is where the constant and dictionary memory wins come from and is allocation-neutral for flat data. The existing compressed-string append path (`ColumnDataCopyCompressedStrings`) kept its dictionary per 2048-row block and lost the compact form after one collection-to-collection hop. Where recognition applies it is subsumed. Where it cannot apply (client result collections and appends into entries that already hold flat rows) it is retained as the fallback, so repeated long-string bytes stay shared there. ### Measurements reldebug, 20M rows, medians: | Shape | Wall | Peak RSS | Sink CPU | |---|---|---|---| | materialized CTE, low-NDV strings | 0.106 -> 0.097 s | 573 -> 318 MB | 0.62 -> 0.32 s | | same, short inlined strings | 0.106 -> 0.094 s | 536 -> 307 MB | 0.63 -> 0.31 s | | collection-to-collection hop | 0.165 -> 0.114 s | 1494 -> 563 MB | 2.54 -> 0.77 s | | constant columns (int, short and long strings) | 0.28-0.45 -> ~0.20 s | ~370-530 -> ~215 MB | at the flat-int baseline | Protection shapes (unique strings, enum keys, flat baselines, ordered windows, cross products) measure parity. On TPC-H and TPC-DS (sf1 and sf10): TPC-H gains nothing, its collection-materializing operators are 1.2% of suite CPU and carry numeric payloads. Both suites feed collections from aggregates and sorts, so appended vectors arrive flat and recognition does not fire. The dedup half applies to TPC-DS q47, q57 and q67, which window over low-NDV strings: thread sampling attributes 20-25% of q67's busy CPU to string appends into the window collection, about 4% of suite CPU combined, and peak buffer usage roughly halves on all three at both scale factors (q67 at sf10: 3.75 to 1.68 GB, bounded at -24% wall by an ENUM pre-cast proxy that is loose on time). Ordered windows sort before materializing, so the window shape itself is parity. The wins concentrate at materialized-CTE and hop shapes these benchmarks under-represent, and the suites bound the regression risk: the flat path costs them nothing. ### Consumers Four consumers read `ALLOW_ZERO_COPY` scans through flat accessors and flatten at their boundary: parquet writer row-group scans, the deprecated C API result translation, the box renderer, and `WindowCursor`. The MODE and quantile window aggregates use their own cursors over the window collection and flatten there as well. `ColumnDataConsumer` handles chunks without blocks, since all-constant chunks no longer allocate any. ### Scope Nested types still flatten at append, so constant lists and structs keep today's per-append copy behavior. That is where the shared-list mechanism re-lands as a third representation class. Exchange-side preservation (pipeline chunk buffering flattens vectors between pipelines, which keeps single-hop CTE consumers at parity) and per-segment dictionary hash caching at scan handoff are follow-ups. ### Tests sqllogictests cover constants of all major types including NULL interleavings and 50 KB strings, storage dictionaries (dict and dict_fsst, NULL-bearing and inlined modes) through CTEs, hops, windows, cross products, DISTINCT, recursive CTEs and UPDATE, a spill test with identical results under a memory limit, and the eligibility gate boundaries (dictionary size at the vector size and one past it, code width, heap exactly at and one byte past the block size). A C++ test covers what SQL cannot observe: scan-handoff vector types, per-segment dedup counts, representation survival across a hop, the DISALLOW flat guarantee, lifetime of copies after the source collection is destroyed, and block relocation repairing dictionary and constant string pointers under both scan policies. |
||
|
Mark
|
8bf2888bcb |
Optimise some hot loops (#27905)
Went on a bit of a quest to optimise some hot loops: - Add shared batched Boolean selection for expressions and native/Parquet dictionaries. - Compact semi/anti and outer-join matches in hash and nested-loop joins, including build-side scans. - Hoist invariant accesses out of hash probing and row scatter/gather. - Batch dense integer filtering and bitpacked decoding. - Avoid redundant scan counting and Bloom-filter NULL checks. - Inline floating-point comparisons. - Fix mapped true/false selection handling. This was incredibly fruitful; I'll let the CI regression tests speak for themselves. |
||
|
Mark
|
d63190bb7e |
Restore iceberg for the Wasm platform (#27941)
https://github.com/duckdb/duckdb/pull/27532 temporarily removed Wasm support, I don't think that's intended or necessary, let's check what was the original issue. |
||
|
Sander Mathijs van Veen
|
327b525a1f | Reduce extension CI workload by ignoring changes to /extension/** | ||
|
Laurens Kuiper
|
ae1d827031 | Address distinctness statistics review feedback | ||
|
Robinho-MR
|
439dcf0410 | Merge remote-tracking branch 'origin/main' into scalar-function-fixes | ||
|
Robinho-MR
|
0c594749f4 | Merge remote-tracking branch 'origin/main' into timestamp-rendering-fixes | ||
|
Robinho-MR
|
42a9eae6cc | Merge remote-tracking branch 'origin/main' into parquet-file-fixes | ||
|
Robinho-MR
|
7bb58246e3 | Merge remote-tracking branch 'origin/main' into scalar-function-fixes | ||
|
Robinho-MR
|
1814242d76 | Merge remote-tracking branch 'origin/main' into timestamp-rendering-fixes | ||
|
Robinho-MR
|
c35886da04 | Merge remote-tracking branch 'origin/main' into parquet-file-fixes | ||
|
Robinho-MR
|
80b26fe71f | Merge remote-tracking branch 'origin/main' into scalar-function-fixes | ||
|
Robinho-MR
|
12de1fe85a | Merge remote-tracking branch 'origin/main' into parquet-file-fixes | ||
|
Robinho-MR
|
a2966b1c20 | Merge remote-tracking branch 'origin/main' into timestamp-rendering-fixes | ||
|
Mark
|
d1de96c1eb |
Warn when any deprecated setting is set, including extension settings (#27411)
#25330 introduced `is_deprecated` with "Deprecated settings will show a warning when used", but the flag only filters `duckdb_settings()`. The warning came from a callback written for each setting, so the two drifted apart: - `delim_join_as_cte`, `error_on_division_by_zero`, `experimental_metadata_reuse`, `force_column_metadata_reuse`, `legacy_disable_null_type`, `regex_match_operator_semantics` and `table_function_identifier_conversion` warned but were still listed by `duckdb_settings()`. - `lambda_syntax` and `vacuum_rebuild_indexes` were flagged but never warned. - Extension settings registered with `AddExtensionOption(..., is_deprecated = true)` could not warn; the helper was static in core. **The change** - `SettingCallbackInfo::WarnDeprecated` logs "The 'x' setting is deprecated and will be removed in a future release." It is called by `SET` (core and extension options) and by the config path for every option flagged `is_deprecated`, after the value is validated, and not on `RESET`. - The seven settings above are flagged in `settings.json`, and the callbacks that only warned are removed. `regex_match_operator_semantics` and `table_function_identifier_conversion` keep their validation. - `profiling_mode` keeps its own, more specific warning. The warning remains a log entry, as before: it is visible with logging enabled, and an error under `warnings_as_errors`. Whether it should reach users without logging is left for a follow-up. It fires after the setting's callbacks, which may still refuse the value, so a refused value does not warn; under `warnings_as_errors` the callbacks have run by the time the statement fails, as before. A deprecated setting in the startup config does not warn: core settings are applied before a database exists, and extension settings before logging can be enabled. `vacuum_rebuild_indexes` can only be set there, so it is listed as deprecated but never warns. **Verification** - `test/sql/settings/deprecated_settings.test` gains `lambda_syntax`. - `test/api/test_config.cpp` sets every setting `duckdb_settings(deprecated := true)` lists and expects the warning, so the list and the warnings cannot drift apart again, and checks a deprecated extension option warns on `SET`, not on `RESET`, and is listed only with `deprecated := true`. - The default unit test set passes. |
||
|
Mark
|
12cafe755d |
Codename 'Islandica' (#27947)
Adds the release codename for the upcoming 2.1 series to `DuckDB::ReleaseCodename()`: builds tagged `v2.1.*` now report `Islandica` from `PRAGMA version` / `pragma_version()` instead of falling through to `Unknown Version`. Same shape as the previous codename additions (#23056 for Cyanoptera). No test changes: `test/sql/pragma/test_pragma_version.test` only asserts a codename on dev builds and `v0.0.1`. Note: this PR was generated by an AI coding agent and reviewed by the author. |
||
|
Mark
|
142a9e7dbc |
Fix DETACH in a transaction, instance cache reuse, encrypted attach, logging storage and extension installs (#27765)
Fixes problems around database and extension lifecycle: DETACH of a database used by the current transaction, handing out cached databases that are still in use, the storage version of encrypted databases attached without one, the path of file log storage, and extension installs in progress. Combines the PRs labeled `Ready To Merge` in https://github.com/duckdb-bot/duckdb/pulls below, on top of the current head of `main`. The changes of each PR are applied as one squashed commit. ## Changes applied (5 PRs) - duckdb-bot/duckdb#195: Allow DETACH of a database the current transaction has used, so its COMMIT commits the work - duckdb-bot/duckdb#110: Hand out a cached database that a connection or a result still holds after its last handle is closed - duckdb-bot/duckdb#213: Keep the storage version of an existing encrypted database when attaching it without one - duckdb-bot/duckdb#146: Keep the path of file log storage when enable_logging fails, runs prepared after a switch, or names the storage in uppercase - duckdb-bot/duckdb#14: Skip extension installs in progress when listing and updating extensions ## Original descriptions <details><summary>duckdb-bot/duckdb#195: Allow DETACH of a database the current transaction has used, so its COMMIT commits the work</summary> Since 1.5.3, `DETACH` after reading or writing an attached database fails with `Cannot detach database "other_db" because the current transaction has outstanding work on it` (duckdb/duckdb#23374, duckdb/duckdb#25223). The error aborts the transaction, so the following `COMMIT` silently rolls back all its work. The same sequence committed in 1.5.2. duckdb/duckdb#22435 added the `PhysicalDetach` check because reusing a detached name caused a fuzzer crash in `MetaTransaction::UseDatabase`. That crash is no longer possible: `DETACH` removes the name from the transaction's used databases while the transaction keeps the database alive, and since `fd5e7bd0d60`, a reused name raises a transaction error. We omit the re-`ATTACH` rejection suggested in duckdb/duckdb#23390 and implemented by the open duckdb/duckdb#25244 because `ATTACH OR REPLACE` would still replace a used database. Review also found that rolling back an `ATTACH` removed whichever database held the name, including a later attachment from another connection. Removing the check lets `COMMIT` commit and checkpoint the detached database and `ROLLBACK` undo its work; the detach itself remains. Reattaching another file under the name works, with a second write raising the usual single-writer error. `RollbackState` now passes the database to `DatabaseManager::DetachInternal`, which removes the entry only if the name still refers to it. The new `attach_detach_in_transaction.test` covers commits, copies from read-only attachments, name removal, rollback and the two-connection case. It fails at the first `DETACH` on `main`, and at the other connection's query without the rollback fix. Updated fuzzer and attachment tests cover allowed detaches, commit, rollback and name reuse after reads and writes. Attach and transaction tests pass in reldebug and sanitizer builds; fuzzer, storage and catalog tests pass in reldebug except failures also seen on `main`. The full unit suite has failures also seen on `main`, plus `connect_passthrough_quack.test`, which fails alongside other connect tests but passes alone. The last shard passes in parts, with Quack tests run individually to avoid a crash also seen on `main` when they share a process. Fixes duckdblabs/bot-issue-tracker#152 Related: duckdb/duckdb#23374 </details> <details><summary>duckdb-bot/duckdb#110: Hand out a cached database that a connection or a result still holds after its last handle is closed</summary> Closing the last database handle while a connection or unfinished result remained open made the next open through the instance cache (`duckdb_get_or_create_from_cache` in the C API, `DBInstanceCache::GetOrCreateInstance`) spin forever at 100% CPU. In duckdb/duckdb#25965, the reporter opens a file, connects, calls `duckdb_close`, and never returns from the second `duckdb_get_or_create_from_cache`. v1.5.6 behaves the same. This affects clients such as R that release connections lazily. The cache entry holds a `weak_ptr<DuckDB>`, and `config.db_cache_entry` keeps the entry alive until the end of `~DatabaseInstance`. `DBInstanceCache::GetInstanceInternal` in `src/main/db_instance_cache.cpp` treated an expired handle as shutdown in progress and busy-waited for the entry to expire. But a connection or result can keep the `DatabaseInstance` alive without a handle, so nothing was shutting down and the entry never expired. The entry now holds only a `weak_ptr<DatabaseInstance>`. Each lookup locks it and returns a new handle through the existing `DuckDB(DatabaseInstance &)` constructor, so a database kept open by a connection or unfinished result is handed out as if its handle had stayed open. The cache still waits for shutdown only when the instance itself is gone. Since a `weak_ptr` can only be locked while the instance is alive, this cannot return an instance being destroyed or open a second instance on the same file. The configuration check still applies, and `on_create` still runs only when an instance is created. After waiting for shutdown, the cache now erases the key only if its entry has expired. This fixes a race also present on `main`, where one waiting thread could erase another thread's newly created entry, allowing a later lookup to open a second instance. A new case in `test/api/test_instance_cache.cpp` covers connections and unfinished results keeping the database open, configuration rejection, `on_create` running once, closure after the last reference is released, reopening a closed database, and reopening while the last connection closes. A new case in `test/api/capi/v1/test_capi_instance_cache.cpp` runs the reporter's C API scenario and reads the data back through the reopened database. Both hang on `main` and pass here. The new hidden case, "Test reopening a database from several threads while it shuts down", hands out two different instances without the erase fix and passes with it. All instance cache tests, including hidden tests, and the `[api]` and `[capi]` tests pass on reldebug and on relassert with sanitizers. All unit tests pass on reldebug except `test/sql/settings/reset/reset_threads.test`, which also fails on `main` on this machine. No benchmark was run; the change adds one small handle allocation per cache lookup. The reporter's other open pull request, duckdb/duckdb#26134 against `v1.5-variegata`, also tracks the instance but raises a "still in use" error after a grace period instead of handing it out. Fixes duckdblabs/bot-issue-tracker#116 Fixes duckdblabs/bot-issue-tracker#291 Related: duckdb/duckdb#25965 Related: duckdb/duckdb#22277 </details> <details><summary>duckdb-bot/duckdb#213: Keep the storage version of an existing encrypted database when attaching it without one</summary> Attaching an existing encrypted database without `STORAGE_VERSION` used v1.5.0 instead of the file's version. For a v2.0.0 file, `ATTACH 'enc.db' AS e (ENCRYPTION_KEY 'k')` failed with "cannot initialize database with storage version 68 - which is lower than what the database itself uses (69)", requiring `STORAGE_VERSION 'v2.0.0'` on every attach. A v1.4.0 file attached but became v1.5.0 at the next checkpoint, leaving DuckDB 1.4.4 unable to open it. Unencrypted files keep their version. Our fixing agent found this while working on duckdblabs/bot-issue-tracker#652; there is no duckdb/duckdb issue. DuckDB 2.0.0 fails the same way; v1.5.6 cannot write v2.0.0 files. `StorageOptions::SetEncryptionVersion` in `src/storage/storage_manager.cpp` picks the encryption format before it knows whether the file exists. Without `STORAGE_VERSION`, it also set `storage_version` to `StorageVersion::V1_5_0`, the default encryption format's minimum. `SingleFileBlockManager::Initialize` treated this as an explicit request: it rejected newer files and adopted v1.5.0 for older ones, which the next checkpoint wrote into the header. `SetEncryptionVersion` now leaves `storage_version` for user requests only. Existing files use their header's storage version; their encryption version already came from the header. In `SingleFileStorageManager::LoadDatabase`, new encrypted files now follow `storage_compatibility_version`, like unencrypted files: v2.0.0 by default, with a v1.5.0 floor when `storage_compatibility_version` is set below v1.5.0. `test/sql/attach/attach_encrypted_storage_version.test` checks defaults with both encryption formats, the floor, reattaches, explicit downgrade rejection and v1.4.0 preservation after a write and checkpoint. `test/sql/attach/attach_encrypted_v1_4_file.test` checks that an encrypted file written by DuckDB 1.4.1 stays v1.4.0 after a write and checkpoint; a manual check confirmed DuckDB 1.4.4 still reads it. Both tests fail on main and pass in the release build with debug info and the build with sanitizers and assertions. Removing 49 skips from `test/configs/encryption.json` lets 48 tests requiring v2.0.0 run and pass; one remains skipped by `on_load`. All unit tests pass in the release build with debug info, and the fast unit tests pass under `test/configs/encryption.json` using `force_mbedtls_unsafe` instead of httpfs, both except `test/sql/settings/reset/reset_threads.test`, which also fails on main because the suites ran with `unittest --max-threads`. The attach, encryption, storage and serialization tests pass in the sanitizer build. The separate problem where an encrypted file created with `STORAGE_VERSION 'v1.4.0'` gets the newer encryption format and cannot be opened by DuckDB 1.4 remains unchanged, tracked in duckdblabs/bot-issue-tracker#654. Storage change: New encrypted databases without `STORAGE_VERSION` now default to v2.0.0 instead of v1.5.0, like unencrypted databases; existing files keep their version. DuckDB 1.5.6 and 1.4.4 reject these v2.0.0 files with "created with a newer version of DuckDB", DuckDB 2.0.0 still requires `STORAGE_VERSION 'v2.0.0'` to open them, and `STORAGE_VERSION 'v1.5.0'` or `storage_compatibility_version` v1.5.0 keeps new files readable by DuckDB 1.5. Fixes duckdblabs/bot-issue-tracker#653 To clarify, I'm an autonomous bot following workflows described by the DuckDB maintainers. Take what I say and do with a grain of salt. You can find more information, or report concerns about my behavior, in this repository: https://github.com/duckdb-bot/guidelines agent_version:8ca2944e36c75c348a14af69198598982374be98 </details> <details><summary>duckdb-bot/duckdb#146: Keep the path of file log storage when enable_logging fails, runs prepared after a switch, or names the storage in uppercase</summary> Three `enable_logging` cases left file log storage without a path: a failed switch from file storage using `CALL enable_logging(storage = 'memory', storage_config = {'delim': '|'})`, a prepared `enable_logging(storage = 'file')` executed after switching to memory storage, and `CALL enable_logging()` after enabling file storage with `storage = 'FILE'`. Every subsequent statement failed with `Invalid Configuration Error: Failed to initialize file log storage table, path wasn't set`, and the process aborted on exit. We found these while fixing duckdblabs/bot-issue-tracker#516. On partial failure, `enable_logging` in `src/function/table/system/logging_utils.cpp` restored the previous `LogConfig` through `LogManager::SetConfig`. A `LogConfig` names the storage but omits its storage config, so this created a new `FileLogStorage` without a path. The guard against file storage without a path ran only at bind time, which a prepared call skips. `SetConfig` also kept the name `FILE`, while `SetLogStorageInternal` compared against the lowercased name, causing the next call to replace the storage. The change adds `LogManager::GetConfigAndStorage` to save config and storage together, and `LogManager::RestoreConfig` to restore the config and storage object itself on failure. The path guard, now `VerifyFileStoragePath`, also runs at execution, and `SetConfig` keeps the name lowercased by `SetLogStorageInternal`. `test/sql/logging/enable_logging_file_keeps_path.test` covers all three cases, checks that statements keep working and which storage is active, and fails on `main`. Logging tests pass on reldebug and on relassert with sanitizers; all reldebug unit tests pass except failures also present on `main` on this machine. No benchmark was run because only `enable_logging` changes. A storage switch by another connection between the execution-time check and the change itself remains unaddressed. Draft duckdb-bot/duckdb#139 (duckdblabs/bot-issue-tracker#516) fixes other routes to the same error in the same files. Both change the path guard in `logging_utils.cpp`, so the PR merged second needs a merge. Fixes duckdblabs/bot-issue-tracker#581 </details> <details><summary>duckdb-bot/duckdb#14: Skip extension installs in progress when listing and updating extensions</summary> Moved from duckdb/duckdb#27324. Running `SELECT * FROM duckdb_extensions()` or `UPDATE EXTENSIONS` while another process installs an extension into the same directory can fail with a conflicting lock error or a message that an info file appears to be empty. An install first writes `<name>.duckdb_extension.tmp-<uuid>.duckdb_extension` and its `.info` file, then moves both into place. Both scans pick up every file ending in `.duckdb_extension`, including these temporary files. As a result, they can try to read the temporary `.info` while the installer still holds its write lock or before it has written anything. Both scans now use `ExtensionHelper::IsInstalledExtensionFile` to skip temporary install files. The regression test creates a temporary extension, first without an `.info` file and then with an empty one, and checks that neither is listed. It also checks that `UPDATE EXTENSIONS` succeeds with no rows and that an installed extension alongside the temporary files is still listed. The test fails on `main` and passes with this change. Fixes duckdblabs/bot-issue-tracker#46 Related: duckdb/duckdb#26875 </details> |
||
|
Mark
|
7ee6e53e91 |
Push view filters below delim joins and plan many UNION ALL subqueries in linear time (#27760)
Improves planning of queries with subqueries: filters on a view are pushed below the SINGLE and MARK delim joins of its subqueries, and a UNION ALL of many uncorrelated subqueries is planned in linear time. Combines the PRs labeled `Ready To Merge` in https://github.com/duckdb-bot/duckdb/pulls below, on top of the current head of `main`. The changes of each PR are applied as one squashed commit. ## Changes applied (2 PRs) - duckdb-bot/duckdb#65: Push a filter on a view below the SINGLE and MARK delim joins of its subqueries - duckdb-bot/duckdb#212: Plan a UNION ALL of many uncorrelated subqueries in linear time ## Original descriptions <details><summary>duckdb-bot/duckdb#65: Push a filter on a view below the SINGLE and MARK delim joins of its subqueries</summary> Filtering a view with a correlated subquery in its select list ran the subquery for every table row first. With 2,000,000 `facts` rows, `WHERE customer_id = 7 AND id = 1007` took 3.378 s on `main`, versus 0.009 s in v1.5.6. Moving the filter below the subquery took 0.003 s. `PushEligibleFilterExpressionsIntoDelimJoinInputs` moved filters through projections into the left input only for `INNER` and `LEFT` delim joins. Scalar subqueries use `SINGLE`, while select-list `EXISTS` and `IN` use `MARK`, so the filter stayed above these joins and their delim CTEs received every row. We now push eligible filters through joins where `PropagatesBuildSide` is false, adding `SINGLE`, `MARK`, `SEMI` and `ANTI`; `OUTER`, `RIGHT`, `RIGHT_SEMI` and `RIGHT_ANTI` still keep filters above. JSON profile tests check that scalar and `EXISTS` queries unnest 2 rows instead of 20,000. Result tests cover `NULL` elements, empty and `NULL` lists, `IN`, `NOT IN` and filters on subquery results. The new microbenchmark drops from 711.1 ms to 3.4 ms; TPC-H SF1 q02, q04, q17, q20, q21 and q22 are unchanged. Two limitations remain for later. As in v1.5.6, the filter stops above the table scan inside the delim CTE, so every row group is still read. Separately, with a second correlated subquery, the filter passes only one of the two delim joins: that query takes 2.8 s versus 5.7 s on `main`. Related: duckdblabs/bot-issue-tracker#67 Related: duckdb/duckdb#26510 </details> <details><summary>duckdb-bot/duckdb#212: Plan a UNION ALL of many uncorrelated subqueries in linear time</summary> `UNION ALL` queries with many branches, such as those statistics libraries generate for every pair of columns (duckdb/duckdb#22737), spent almost all their time planning, with roughly quadratic growth. The timings use a one-row table of `n` DOUBLE columns, with one subquery for every ordered pair of distinct columns. Each subquery has four CTEs that rank the two columns with `row_number()`, `count(*) OVER ()` and `lag()`; the branches are combined with `UNION ALL` and the outer query counts the rows. 90, 380, 870, 1,560 and 3,540 branches correspond to 10, 20, 30, 40 and 60 columns. A release build of `main` took 0.075, 0.44, 1.57 and 4.91 s for 90, 380, 870 and 1,560 branches, respectively, and about 100 s for 3,540 branches. Version 1.5.6 scales the same way but is about twice as fast. `FlattenDependentJoins::DecorrelateSubtreeInternal` runs on every plan, including those without correlated subqueries. For each operator child, it calls `RewriteCorrelatedBindings`, which walks the entire operator and all its children. The parser represents these unions as one set operation with `K` children, making the walks `O(K^2)`. Only references with bindings in `correlated_aliases` can change, so an empty map makes the walk a no-op. Correlated plans still perform these walks and can still plan superlinearly. Slow `EXPLAIN` rendering has a separate cause in the text renderer, tracked in duckdblabs/bot-issue-tracker#655. The change skips the walk when `correlated_aliases` is empty while still invalidating the access cache. Planning after binding at 1,560 branches drops from 3.9 to 0.06 s; query times for the five sizes above become 0.061, 0.26, 0.63, 1.44 and 3.52 s, close to linear. The new benchmark `benchmark/micro/optimizer/union_all_subquery_planning.benchmark`, listed in `.github/regression/micro.csv`, builds the 1,560-branch query text in its load step and runs it with `FROM query(getvariable('union_sql'))`, checking the count of 1,560. Measured with `scripts/regression/test_runner.py` at two threads on release builds of `main` and this branch, the median over 40 paired runs drops from 5.0 s to 1.3 s. Controls remain comparable: `lateral_domain_filter` and `nested_lateral_domain_filter` stay at 1.3 and 2.7 ms, a union of 400 correlated scalar subqueries takes 0.59 s before and 0.53 s after, and correlated `EXISTS` over 5 million rows takes 0.118 s in both. The benchmark supplies the timing regression check because no test measures planning time deterministically. Subquery and optimizer tests pass on reldebug and on an assertion build with sanitizers. The full unit suite passes on reldebug apart from `reset_threads.test`, which also fails on `main` under the runner’s `--max-threads`. Fixes duckdblabs/bot-issue-tracker#266 Related: duckdb/duckdb#22737 </details> |
||
|
Mark
|
649510c208 |
Nested connections (#27264)
Improving on https://github.com/duckdb/duckdb/pull/27040 via nested Connection constructor. This allows: * checking max-depth * checking timeouts * checking interrupts |
||
|
Mark
|
9c85bddb5b |
Use ART index scans for range conjunctions on the indexed column (#27926)
Hi team, I found in the current implementation, index scan is not leverage on `BETWEEN AND`. For example, ```sql memory D CREATE TABLE t AS SELECT i AS id FROM range(1000000) r(i); memory D ALTER TABLE t ADD PRIMARY KEY (id); memory D EXPLAIN ANALYZE SELECT * FROM t WHERE id BETWEEN 100 AND 200; ╭─ Summary ───────────╮ │ Total Time: 0.0008s │ ╰─────────────────────╯ ╭─ Table Scan ─────────────────────────╮ │ Table: memory.main.t │ │ Type: Sequential Scan │ │ Projections: id │ │ Filters: (id >= 100) AND (id <= 200) │ │ Row Groups Scanned: 1 / 9 │ │ 101 rows 0µs │ ╰──────────────────────────────────────╯ ``` In this PR, `ART::TryInitializeScan` matches a pushed-down AND of one lower and one upper bound, so range predicates like `i BETWEEN a AND b or i > a AND i < b` now use the index scan instead of a sequential scan. Followup items - Residual filters for index scans + Let the index return a superset of rows from the range bounds, then apply the remaining predicates to the fetched rows. This would cover `i > 100 AND i < 110 AND i <> 105` - Filters on multiple columns + `TableScanInitGlobal` falls back to a sequential scan when `FilterCount() != 1`, so `a = 1 AND b > 5` cannot use an index on `a`. With residual filters, we could scan the index on `a` and filter on `b`. - Compound indexes + `TryScanIndex` bails out when `UnboundExpressionCount() > 1`. An index on `(a, b)` could serve `a = 1 AND b BETWEEN 10 AND 20` with a prefix-plus-range scan. |
||
|
Mark
|
81469ccc69 |
Push OR of equalities down as an IN filter (#27929)
Hi team, I found in the current implementation index scan is not always leverage for equality check, for example ```sql memory D CREATE TABLE tbl AS SELECT range::INTEGER AS i, range::VARCHAR AS s FROM range(500000); memory D CREATE INDEX idx_i ON tbl(i); memory D CREATE INDEX idx_s ON tbl(s); memory D memory D EXPLAIN ANALYZE SELECT * FROM tbl WHERE i = 5 OR i = 7; ╭─ Summary ───────────╮ │ Total Time: 0.0043s │ ╰─────────────────────╯ ╭─ Filter ────────────────────────────────╮ │ Expression: (i = 5) OR (i = 7) │ │ 2 rows 0µs │ ╰────────────────────┒┎───────────────────╯ ╭─ Table Scan ───────┚┖───────────────────╮ │ Table: memory.main.tbl │ │ Type: Sequential Scan │ │ Projections: i, s │ │ Filters: optional: ((i = 5) OR (i = 7)) │ │ Row Groups Scanned: 1 / 5 │ │ 122,880 rows 0µs │ ╰─────────────────────────────────────────╯ memory D EXPLAIN ANALYZE SELECT * FROM tbl WHERE i IN (5, 7); ╭─ Summary ───────────╮ │ Total Time: 0.0017s │ ╰─────────────────────╯ ╭─ Filter ─────────────────────────╮ │ Expression: (i = 5) OR (i = 7) │ │ 2 rows 0µs │ ╰─────────────────┒┎───────────────╯ ╭─ Table Scan ────┚┖───────────────╮ │ Table: memory.main.tbl │ │ Type: Index Scan │ │ Projections: i, s │ │ Filters: optional: (i IN (5, 7)) │ │ Row Groups Scanned: 0 / 5 │ │ 2 rows 0µs │ ╰──────────────────────────────────╯ ``` In the current implementation, an OR of equalities on one column (`col = a OR col = b`) was pushed into the scan as an optional OR filter, which the index scan does not recognize, so it fell back to a sequential scan while the equivalent `col IN (a, b)` used the index. |
||
|
Mark
|
afe3784fca |
Make json_structure iterative instead of recursive (continues #27392) (#27897)
Picks up #27392 by @sebastiaan-dev (all original commits are preserved) and rebases it onto current `main`. - Merges `main` into the branch. `main` added `case_insensitive_keys` to the JSON structure nodes (#26817), so those changes are ported to the code moved into `json_schema_inference.cpp`. - `test_json_structure_deep.test` (added on `main`) expected the old recursion-depth error; updated to expect the result, like the other `json_structure` depth tests in the original PR. Original description: makes `json_structure` iterative instead of recursive, and moves the `read_json` schema inference into `json_schema_inference.[hpp|cpp]`. Resolves duckdblabs/duckdb-internal#11437. |
||
|
Mark
|
aa6ae42bae |
Count a gzip file's header in the compressed bytes it consumed (#27797)
Previously the initial gzip header did not go through ReadData so the `compressed_bytes_read` and `compressed_bytes_consumed` for the initial header were not counted. This adds a call to add the `data_start` bytes read in the gzip initialize so these bytes are counted towards the totals. ### Test data A new test case (`"GZIP progress counts every compressed byte, the header included"` in `test/common/test_gzip_file_system.cpp`) was added to check this. Previously, it would report 509 bytes when it should be 529 bytes and fail. With this change, this new test does pass. |
||
|
Mark
|
298732ceb0 |
Add functions to add, list and remove grammar extensions (#27289)
Fix https://github.com/duckdb/duckdb/issues/27224
`SET active_grammar_extensions` replaces the list wholesale, so an
extension cannot enable its grammar without knowing which other grammar
extensions the client has turned on, and vice versa. In the CLI this
meant `SET active_grammar_extensions=['shell']` silently dropped any
grammar extension that activated itself from `OnConnectionOpened`, and a
user activating their own extension with `SET` lost bare-expression
statements.
This PR adds an append/remove form:
```
CALL add_grammar_extension('pipe_query_syntax'); -- appends, no-op if already active
CALL remove_grammar_extension('pipe_query_syntax'); -- removes, keeps the rest
FROM duckdb_grammar_extensions(); -- new `active` column
```
- Both functions go through `ActiveGrammarExtensionsSetting::SetLocal`,
so validation, lock_configuration and grammar caching are shared with
`SET`. Names are case insensitive. Unknown names raise an error with a
"Did you mean" suggestion.
- The CLI now runs `CALL add_grammar_extension('shell')` instead of the
SET, so extensions that activate themselves on the connection stay
active.
- `duckdb_grammar_extensions()` gains an active BOOLEAN column instead
of adding a separate list function.
```sql
DuckDB v2.0.0-dev88461 (Development Version,
|
||
|
Mark
|
ba3b07bffb |
Improved error reporting for the parser (#27387)
The PEG parser reported every syntax error as `syntax error at or near
"<token>"`, using the furthest token any alternative reached. This PR
keeps that message as the first line and, when it can say something
useful, adds one hint line below it. Hints are computed only after a
statement has failed to parse, so valid input pays nothing.
## Hints
Every hint is verified against the grammar before it is shown: a
correction or insertion it only suggested if re-parsing the statement
with that change makes the parser get past the original failure.
### Did you mean "KEYWORD"?
A word that is one or two edits away from a keyword the grammar accepts
at that point (Damerau-Levenshtein, one edit for words of up to four
letters). Keywords that would have been accepted as an identifier at
that position are never suggested, so an alias such as `alias` is not
"corrected" to `ALWAYS`.
```sql
SELECT * FROM t WHERE x = 1 GRUOP BY x;
Parser Error: syntax error at or near "GRUOP"
Did you mean "GROUP"?
```
A misspelled keyword is often swallowed as an alias, so the parser fails
one token late. The hint looks one token back in that case and reports
the error on the misspelling itself. Before, `SELECT a FORM t` was
reported at `t`.
```sql
SELECT a FORM t;
Parser Error: syntax error at or near "FORM"
Did you mean "FROM"?
LINE 1: SELECT a FORM t;
^^^^
```
Other examples: `SELEC 1` -> `SELECT`, `WERE` -> `WHERE`, `LIMT` ->
`LIMIT`, `TABEL` -> `TABLE`,
`VALEUS` -> `VALUES`, `HAVNG` -> `HAVING`, `WHRE` -> `WHERE`.
### Missing "TOKEN"
Given where the grammar accepts only literal tokens, each expected
keyword, comma or closing bracket is inserted at the failure point in
turn. If exactly one of them makes the whole statement parse, it is
reported as missing.
```sql
SELECT (1 + 2 FROM t;
Parser Error: syntax error at or near "FROM"
Missing ")"
LINE 1: SELECT (1 + 2 FROM t;
^^^^
```
```sql
SELECT CASE WHEN x THEN 1 FROM t; -> Missing "END"
SELECT a b c FROM t; -> Missing ","
SELECT * FROM t ORDER x; -> Missing "BY"
SELECT [1, 2 FROM t; -> Missing "]"
SELECT {'a': 1 FROM t; -> Missing "}"
SELECT * FROM t WHERE a IN (1, 2 -> Missing ")"
```
The insertion search is skipped where an identifier or expression is
accepted: a missing identifier cannot be guessed, and a keyword that
happens to parse there is rarely what was meant (`LIMIT ALL` for `...
LIMIT;`).
### Expected ... / Expected one of: ...
What the grammar accepts at the failure point, gathered by re-running
the matcher with an autocomplete cursor there. This reuses the existing
autocomplete suggestion walk.
```sql
SELECT * FROM t WHERE
Parser Error: syntax error at end of input
Expected an expression
```
```sql
SELECT a::; -> Expected a type
SELECT * FROM t WHERE a = 1 AND OR x = 2; -> Expected an expression
SELECT * FROM t AS; -> Expected an identifier
SELECT * FROM t ORDER BY a LIMIT; -> Expected one of: "ALL", an expression
SELECT * FROM; -> Expected one of: "(", "LATERAL", an identifier,
a catalog name, a schema name,
a table function name, a table name
CREATE TABLE t2(; -> Expected one of: ")", "CHECK", "CONSTRAINT",
"FOREIGN", "PRIMARY", "UNIQUE", an identifier
```
Two rules keep this readable:
- Keywords that the identifier positions at that point accept as
identifiers are left out,
using the same keyword-category flags `IdentifierMatcher` uses.
- Whole sub-grammars are summarised by a label. `rule_labels` in
`scripts/parser/grammar_types.yml` maps the expression precedence chain
to "an expression" and `Type` to "a type"; the `LabelRule` calls in
`matcher_factory.cpp` are generated from it and unknown rule names fail
generation.
If the list would still exceed eight entries and no label summarises it,
no hint is printed. `SELECT 1 2` stays a plain `syntax error at or near
"2"`.
### syntax error at end of input
When the statement ends where more input was required, the first line
now says so, as PostgreSQL does, instead of naming the last token that
happened to be there. The caret still points at that last token.
|
||
|
Louie Duck
|
339cf6fc65 | Merge remote-tracking branch 'upstream/main' into pad-without-per-character-decode | ||
|
Mark
|
ec2836c43f |
Don't remove the index when rolling back a comment on it (#27902)
Rolling back a transaction that ran `COMMENT ON INDEX` removes the index from its table. The catalog still lists the index, but the table no longer maintains it, so a UNIQUE index silently stops rejecting duplicates. Once the table has changed, the next checkpoint writes the index entry without its storage, and the database can no longer be opened: ```sql CREATE TABLE t (i INTEGER); CREATE UNIQUE INDEX idx ON t (i); BEGIN; COMMENT ON INDEX idx IS 'x'; ROLLBACK; INSERT INTO t VALUES (1), (1); -- succeeds CHECKPOINT; -- reopening the database fails with: -- INTERNAL Error: ExtractIndexStorageInfo: index storage info with name "idx" not found ``` `COMMENT ON INDEX` replaces the index entry by a copy that shares the index of the entry it replaces and keeps its OID. Rolling back the transaction calls `DuckIndexEntry::Rollback` on that copy, which removes the index with that OID from the table, as it does when undoing a `CREATE INDEX`. `DuckIndexEntry::Rollback` now leaves the index alone when the entry it rolls back to has the same OID, and only removes the index when it undoes its creation. Could this get the `backport v2.0-cyanoptera` label? |
||
|
Laurens Kuiper
|
b81ce92a56 |
Merge upstream main into hot_loops
Preserve batched DICT_FSST Boolean selection alongside the new bounds assertions and validated segment layout. |
||
|
Laurens Kuiper
|
52fd370027 | Use temporary tuple tables in distinctness optimizer tests | ||
|
Laurens Kuiper
|
766da89776 |
Restrict recursive partitions of CTE-decorrelated lateral joins (fixes #27072) (#27089)
Fixes https://github.com/duckdb/duckdb/issues/27072. The issue reports that a selective correlated LATERAL query over a recursive CTE computes every recursive partition on v2.0, while 1.5.6 computes only the matching one. Both versions return the correct result, but with 1000 input rows the recursive CTE emits 3 rows on 1.5.6 and 3000 on v2.0. The report uses a computed STRUCT field as the correlation key, but while investigating it turned out that the struct is incidental. Plain-column variants, nested laterals, and correlated EXISTS all show the same regression, so this addresses the whole class rather than the reported query alone. The root cause is a structural difference between the two decorrelation strategies. The 1.5.x delim joins fed the duplicate-eliminated outer keys directly into the inner plan, where they restricted the correlated input before the recursive CTE ran. CTE-based decorrelation materializes the correlated input as a delim CTE instead, and the selective join against the outer keys sits above that materialization boundary, which no later optimizer crosses. The CTE join filter pusher exists to bridge exactly this gap by planting a semi join against the key domain inside the CTE definition, but it only recognized join conditions whose sides were bare column references and whose key domain was another materialized delim CTE. The reported query misses the pattern twice: the condition compares struct_extract expressions, and the key domain sits behind the duplicate-elimination aggregate. The pass is generalized in two ways. Join keys may now be deterministic single-column expressions, composed through projections and aggregate groups on both the source and target paths, which covers the struct field. The key domain may also be a plain copy-safe subtree of base-table scans, filters, and projections, duplicated via LogicalOperatorDeepCopy, because flattening often simplifies the outer delim join away entirely and leaves no materialized domain to point at. Per review, the scope is kept deliberately small for the bugfix release. Keys are not translated across join equivalences, sources are not narrowed through joins, and only inner joins are collected from, so deeper nested laterals, multiple independent recursive laterals, and correlated EXISTS keep their current plans. Those shapes are also regressions against 1.5.6 and are left to the cost-based domain placement planned for v2.1 (#24561, #27426). Copied sources are additionally gated on the unfiltered scan estimate, since the filtered output cardinality alone would admit a second scan of a large table that filters down to a few rows. Keys whose expressions can throw, such as overflow-checked arithmetic or lossy casts, are not pushed, matching filter pushdown conservatism. The regression test extends test/optimizer/pushdown/cte_domain_join_filter.test with the reported query and related shapes. The restricted cases assert the 1.5.6 partition counts (for example 3 instead of 3000 recursive rows for the reported query). The deferred shapes keep result assertions with unrestricted partition counts, marked for re-tightening by the v2.1 work. A further test asserts that a large filtered source is scanned only once. |
||
|
Laurens Kuiper
|
b9089f04f5 | Keep deliminator anti-join test ranges overlapping | ||
|
Denis Hirn
|
5e70475b1c |
Lower the quantile build OOM limit below the compact-argument peak
The windowed quantile materializes its folded constant fraction argument into the window collection. With compact constant entries that column costs metadata instead of 34MB of repeated doubles, so the build fits under the test's 400MB limit in the persistent-storage configurations, where the base table blocks are evictable, and the expected Out of Memory no longer fires (release thresholds: flat 440MB, compact 400MB). 360MB keeps the build failing with the compact representation while the subset queries and the mad() section are unaffected. |
||
|
Denis Hirn
|
70ab61acab |
Use the current sqllogictest path keyword spelling
The merge queue failed in the prepare format check: main now rewrites
the deprecated __TEST_DIR__ spelling to {TEST_DIR} (#27605, landed
after this branch's last CI pass), and two branch test files still used
the old form. The runner accepts both, so only the spelling changes.
|
||
|
Denis Hirn
|
831d21dc22 |
Cover the windowed string skip list across flat pages
The dictionary-page window test only reaches the skip list through compact pages. This section pins the flat-page half of the ownership contract: unique non-inlinable strings spanning several collection blocks, a trailing frame crossing block boundaries, and exact per-row expectations for the scalar and list quantile forms. Fails on the merge base in the Destroy Unpinned Blocks configuration with the skip-list ValueError and passes with the owned-value fix. |
||
|
Denis Hirn
|
7822c67ef0 |
Read collection pages in place in the IEJoin cursor and the width pass
Review follow-up: two remaining scan consumers read FlatVector::GetData straight off a ColumnDataCollection scan and relied on their producers only ever appending flat vectors. IEJoinCursor now holds a VectorIterator rebuilt per page load, the same shape as the MODE and quantile window cursors, and the box renderer width pass reads through Values<uint64_t>() like the value pass. Both producers still append flat today, so this hardens the consumers without behavior change. |
||
|
Denis Hirn
|
0ca8b0d592 |
Own string values in the windowed quantile skip list
The skip list retains cursor values across page loads, but for VARCHAR inputs those values are string_t pointers into the current page of the window collection scan. Re-paging unpins the previous blocks, so the retained entries dangle and a later remove() cannot find them, escaping as a skip-list ValueError (what() = "std::exception"). Caught by the Destroy Unpinned Blocks lane on the dictionary-page window test, and reproduced on the merge base with flat pages, so the bug predates the compact representations and needed string data spanning multiple blocks plus a frame crossing a block boundary to surface. QuantileSkipValue stores strings as owned std::string in the skip list nodes and passes every other type through unchanged. Extraction hands out views into the state-owned skips vector, which QuantileCast copies into the result vector immediately. This mirrors ModeState, which already deep-copies its string keys through OwningStringMap. |