eldenmoon opened a new pull request, #68447:
URL: https://github.com/apache/doris/pull/68447
### What problem does this PR solve?
Issue Number: None
Related PR: #67675
Problem Summary:
Whether a VARIANT JSON parse error is reported depended on two mutable BE
configurations, not on the function that was called:
- `variant_throw_exeception_on_invalid_json` (default `false`) stored text
that is not JSON as a VARIANT string. `PARSE_TO_VARIANT('hello')` returned the
string `hello`, and `TRY_PARSE_TO_VARIANT` never returned NULL for such text.
Load jobs parse string fields with `TRY_PARSE_TO_VARIANT`, so they behaved the
same way.
- `variant_enable_duplicate_json_path_check` (default `false`) made a
repeated key an error. Set to `true`, it kept the first value, both when
parsing and when a dotted key and a nested path collided in storage. Its name
reads as the opposite of what it does.
- An empty string parsed as an empty object `{}`.
This PR removes both configurations. `variant_max_json_key_length` stays.
The error now depends only on the function:
- `PARSE_TO_VARIANT` fails, and `TRY_PARSE_TO_VARIANT` and load jobs return
NULL, for:
- text that is not a JSON document, including an empty string;
- a repeated key;
- a key longer than `variant_max_json_key_length`;
- nesting deeper than 128 levels;
- invalid UTF-8.
- A dotted key that collides with a nested path when the value is stored
always fails the write (`may contains duplicated entry`). This was already the
default; there is no longer a way to keep the first value instead.
- The Variant serde still keeps text it cannot parse as a string, and it
accepts keys of any length. It mostly re-reads JSON that a Variant serde wrote,
for example `COLLECT_LIST` aggregate states, so it must read back every value a
Variant can hold. The current simdjson DOM parser cannot read back an integer
beyond 64 bits, so a strict re-read would fail such queries. The leniency for
invalid text can be removed once big integers parse.
Behavior before and after:
| Input | Before | After |
| --- | --- | --- |
| `PARSE_TO_VARIANT('hello')` | string `hello` | error |
| `TRY_PARSE_TO_VARIANT('')` | `{}` | NULL |
| `PARSE_TO_VARIANT('{"a":1,"a":2}')` | error; `{"a":1}` with the config set
| error |
| CSV load, field `not-json` / empty field | string / `{}` | NULL / NULL |
| JSON-format load, `{"v": "hello"}` / `{"v": ""}` | string / `{}` | NULL /
NULL |
| JSON-format load, `{"v": "123"}` / `{"v": "{\"a\":1}"}` | `123` /
`{"a":1}` | unchanged |
| A row that loads NULL into a `NOT NULL` VARIANT column | the text was
stored | filtered, counted in `max_filter_ratio` |
A JSON-format load passes a string value to the parser without its quotes,
which is how a string holding JSON text gets parsed. As a result a plain string
value such as `"hello"` now loads NULL. JSONB columns already behave this way,
because they are loaded through `jsonb_parse_error_to_null`.
A `be.conf` that still sets one of the removed configurations starts
normally and logs `Unknown config ... is ignored`; `update_config` for them
returns `NOT_FOUND`. With `strict_mode = true`, rows that parse to NULL are
reported as filtered rows. During a rolling upgrade, old and new BEs apply
different rules.
Until big-integer parsing lands, a document that contains an integer outside
[-2^63, 2^64 - 1] or a number outside the DOUBLE range follows the same rule:
an error for `PARSE_TO_VARIANT`, NULL for `TRY_PARSE_TO_VARIANT` and loads.
Before this PR, the whole document was stored as a string.
### Release note
`PARSE_TO_VARIANT` returns an error, and `TRY_PARSE_TO_VARIANT` and load
jobs return NULL, for text that is not valid JSON, an empty string, or an
object with a repeated key. Previously such text was stored as a VARIANT
string, and an empty string became `{}`. A plain string value in a JSON-format
load now loads NULL. The BE configurations
`variant_throw_exeception_on_invalid_json` and
`variant_enable_duplicate_json_path_check` are removed.
### Check List (For Author)
- Test: Regression test / Unit Test / Manual test
- BE UT `*Variant*:*variant*`: 662 passed.
- Regression `variant_p0`: 173 suites.
- `v2/variant_parse_functions` was regenerated for the new rules.
- `duplicate_json_path` was rewritten: it tested the removed
keep-first mode.
- The new `load_invalid_json` covers JSON and CSV loads and a `NOT
NULL` column.
- `duplicate_json_path` also checks that a dotted-key collision
fails the whole load.
- `doc_mode/test_outfile_csv_variant_type` fails only on S3
credentials in the test environment.
- Regression, 92 other suites that parse or load VARIANT (inverted
index, partial update, JSON load, MV, table functions, and more): 89 passed.
`test_bitmap_filter` and `test_bitmap_filter_nereids` need the `test_query_db`
fixture, and `arrow_flight_sql_p0/test_get_tables_schema` failed on an Arrow
Flight connection error in the test environment.
- Manual: the before/after table was produced by running the same
queries and loads on a cluster built from master plus #67675 and on this branch.
- `build-support/clang-format.sh` and
`build-support/check-build-hygiene.sh` passed.
`build-support/run-clang-tidy.sh` reports nothing on changed lines.
- Behavior changed: Yes (see the table above)
- Does this need documentation: Yes. The VARIANT pages in
apache/doris-website#4170 describe the configurations and will follow this PR.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
