raghav-reglobe commented on code in PR #69222:
URL: https://github.com/apache/doris/pull/69222#discussion_r4552475931
##########
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:
A correction to my reply above. Its paragraph "The call is typed where it is
computed" no longer describes the code.
Since 7eedb15cc53 and ecc7f39278d the template applies to the value the
column stores and to nothing else. In INSERT, UPDATE and MERGE INTO a
`parse_to_variant` / `try_parse_to_variant` call is read with the template only
when the templated column is the one reader of its value. When anything else
reads that value, the call is left as it is: that reader sees what it sees in a
plain SELECT (`1.5` for `1.50`), and the column stores what it stored before
this PR. "Anything else" is a filter, a join or sort key, a GROUP BY, another
column of the table, a generated column or a materialized view over the
templated column, or a second copy of the same call in the statement.
The reason is that a typed call changed more than the column. Every other
reader of the value saw the new read too, and a comparison against a second
parse of the same text stopped matching (`1.50` against `1.5`). A reader that
held its own copy of the call kept the old read: a generated column `g AS
(CAST(v['a'] AS DECIMAL(10,2)))` stored 1.00 beside `v['a']` = 1.01.
A load is not affected. It parses the column once with the template, and its
WHERE and derived columns read that value.
`test_predefine_template_expr_and_merge` pins both sides in the tags
`readers`, `merge_readers`, `generated`, `generated_key`, `mv_base`, `mv_index`
and `load`. The PR description has the full rule.
The rule that decides this, `TypeVariantTemplateParse`, is the largest FE
part of the PR. If you would rather review it on its own, I can move it to a
follow-up PR. This one would then keep the wire type, the two BE text readers
and loads, and an explicit `parse_to_variant` call in a statement would read as
before.
##########
gensrc/thrift/Types.thrift:
##########
@@ -136,6 +136,15 @@ enum TInvertedIndexFileStorageFormat {
SNII = 4 // SNII native inverted index storage format
}
+// One field of a VARIANT Schema Template, in declaration order, for the BE's
JSON parser: a number on a
+// path whose first matching field is DECIMAL (or ARRAY<DECIMAL>) is parsed
from its text instead of
+// through a binary64. The whole template rides so the BE matches a path the
way the shredder does.
+struct TVariantTemplatePath {
+ 1: required string pattern
+ 2: required bool is_glob
+ 3: required bool is_decimal
Review Comment:
Three details of my reply above have changed since:
- "With a DECIMAL field every document is read with the ondemand parser" now
holds for a document that is an object or an array. A root scalar is on no
path, so it is parsed as without a template (25f62d7bac8).
- The three fields of `TVariantTemplatePath` are `optional` now
(abbe5d4d81b). A BE that finds one missing in any entry leaves the whole
template out, because the first matching field decides.
- A CAST does not read by the template. The BE reads it in two places: the
parse functions take it from their result type, and the VARIANT serde takes it
from its type when it reads JSON text (a load, the state of `collect_list`).
The FE takes it off the target of a CAST or TRY_CAST before the expression is
sent (9146a62878f), so a cast of a JSON string to an ARRAY or STRUCT of a
templated VARIANT reads what it read before this PR.
One addition on the BE side: a nonzero number below every scale the field's
type has is read as the 0 it rounds to (7750d214c68, c1a0a8520ca). That is
below 10^-39 on a field of up to 38 digits, and below 10^-77 on a DECIMAL256
field, whose scale goes to 76 and which is told apart for that.
--
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]
