raghav-reglobe commented on code in PR #69222:
URL: https://github.com/apache/doris/pull/69222#discussion_r4546128178
##########
regression-test/data/variant_p0/predefine/sql/q02.out:
##########
@@ -1,6 +1,6 @@
-- This file is automatically generated. You should know what you did if you
want to edit this
-- !q02 --
Review Comment:
Added `sql/q03` to both `variant_p0/predefine` and
`variant_p0/v2/predefine`, with the same cases in each. The tables are created
at the end of `load.groovy`, so `load.out` is unchanged.
Every document is loaded three ways: `INSERT … SELECT parse_to_variant`,
`INSERT … SELECT try_parse_to_variant`, and Stream Load JSON. Six are also
loaded through the `INSERT … VALUES` fast path: 38 fraction digits on
decimal(38,38), 1.005 on decimal(10,2), and 2^64, 2^127−1, −2^127 and 2^127 on
LARGEINT. Each is read back whole, by typed subcolumn, and as text.
- **DECIMAL**
- decimal(38,0) and decimal(38,38) keep full precision.
- On decimal(10,2), extra fraction digits are rounded from the exact text:
1.005 → 1.01 and 0.125 → 0.13.
- 1.5e3 → 1500.00, 15E-1 → 1.50, −0.0 → 0.00, −1.50 stays −1.50.
- 43 significant digits on decimal(38,38) are rounded correctly.
- Overflows give NULL: a 39-digit integer on decimal(38,0), 123456789.123
on decimal(10,2), and 1e400.
- Also covered: ARRAY<DECIMAL>, a glob field before a named one and the
reverse (the first match wins), and a declared path under an array element,
which stays off the path.
- **Big integer on a LARGEINT path**
- Exact: 2^63−1, 2^63, 2^64−1, 2^64, −2^63−1, 10^38−1, 10^38, ±(2^127−1)
and −2^127.
- 2^127 gives NULL.
- 1.5 → 1, as before.
- Also covered: the string form, ARRAY<LARGEINT>, a nested path, a glob
field, and one load that mixes a value below 10^38 with one above it.
- One array mixing an int64 with 2^127−1 reads back exact, `[1,
170141183460469231731687303715884105727]`. 2^127−1 as a number and as a JSON
string, loaded together, read back alike by subcolumn, as text and in the
document.
- **BIGINT path:** 2^63 gives NULL through the cast.
- **Off any template path:** 2^63 loads. 2^64 and 2^127−1 keep today's
behaviour: `parse_to_variant` fails, `try_parse_to_variant` and Stream Load
store a NULL variant (the Stream Load runs with `max_filter_ratio` 0).
- **Doc mode**
(`variant_p0/doc_mode/predefine/test_predefine_largeint_text`, new): doc mode
accepts a LARGEINT template field, so the text read reaches it. Ten documents,
three load ways, one table reading paths from the document and one with
materialized subcolumns. The document column keeps each value as parsed, before
the template's cast: up to 10^38−1 an unquoted number, above that the quoted
digits, the same as the string form. Every LARGEINT reads back exact by
subcolumn, on both tables. 2^127 keeps its digits in the document and reads
back NULL as a LARGEINT; as a string it gives the digits where the path is read
from the document, and NULL where the subcolumn is materialized.
- **Other expressions and statements**
(`variant_p0/predefine/test_predefine_template_expr_and_merge`, new), on a
template with decimal(10,2), decimal(20,9) and LARGEINT paths: the call under
`if`, `case when` (searched and simple), `coalesce`, `nvl` and `nullable`; an
`if` over a subquery's column; `MERGE INTO`, both branches; `UPDATE`; and a
table with `variant_enable_typed_paths_to_sparse`. Each reads back 1.005 → 1.01
and 0.125 → 0.13, 188118222.011121933 whole, and 2^64 and 2^127−1 exact.
I also ran every suite under `variant_p0/predefine`,
`variant_p0/v2/predefine` and `variant_p0/doc_mode/predefine`, and 20 other
`variant_p0` suites over typed templates (types, hierarchical, delete/update,
rqg4/rqg7, compute_v2, topn, load_invalid_json, parse_functions, …), on a
single-FE/single-BE build of this branch. All pass, with these notes:
- `test_variant_multi_index_nonCurrent`,
`test_variant_predefine_types_with_indexes_profile` and
`test_variant_is_null_expr` need `enable_debug_points`, which my build turns on.
- `doc_mode/test_outfile_csv_variant_type` needs S3 credentials my build
doesn't have, so it did not run to completion.
Two tests were wrong at the previous head; both are fixed:
- `TypeVariantTemplateParseTest`: its source table is empty, so
`PRUNE_EMPTY_PARTITION` replaced the scan with an empty relation, and the
projection carrying the call disappeared. The class now disables that rule, as
the other planner tests over empty tables do (4a7f46cb134).
- `q02_13` (`decimal_ >= 188118222.011121933`): row 1 holds exactly that
number and is now returned. The expected output still had the row set from when
the value went through a double (6a68fc45fae).
Also in this push:
- **A bug in the DECIMAL text reader**, found by a local review. It stopped
adding exponent digits at 1000, so an exponent of five or more digits was cut
short. With a long enough fraction the value read was wrong: `0.<999
zeros>1e10000`, which is 10^9000, was read as 1. Fixed in 92d1b34eb69: every
exponent is now read in full until it can no longer change the result. I also
compared both text readers with an exact reference on 300,000 generated tokens;
they agree on all of them.
- **`MERGE INTO`, and a call under `if`, `case when`, `coalesce` or `nvl`**,
were not typed, so they kept the read through a double and still failed on an
integer beyond 64 bits. They are typed now (34736543c9f). `MERGE INTO` wraps
every value it assigns in an `if` of its own, which is why it was missed.
- **The call is typed where it is computed.** Inside a statement that loads
a templated column, a filter, join or sort over the parsed value therefore sees
what the template reads: `cast(v['amount'] as string)` is `1.50` there and
`1.5` in a plain SELECT. The new suite pins this. Say if you would rather keep
such readers on the plain read; that needs a second call for the sink.
- **Under a template without a DECIMAL field the DOM parser now reads
first** (bee6f544dab), so a table whose template declares only LARGEINT fields
pays the text read only for a document the DOM parser rejects. A document
holding an integer beyond 64 bits now costs a failed DOM parse plus the text
read; I have not measured either side. The big-integer table of `q03` has such
a template, and its expected output is unchanged.
--
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]
