Doris parse number fail, string: '-----2628' — root cause analysis & fix

gistfile1.txt

Doris parse number fail, string: '-----2628' — root cause analysis & fix

Context

Inserting CloudFront logs from a Glue/Hive JSON external table into an OLAP unique-key table:

INSERT INTO internal.cloudfront.test
SELECT ..., IF(`sc-content-len` = "-", NULL, `sc-content-len`) AS `sc-content-len`
FROM aws_glue.`cloudfront-logs`.cloudfront WHERE ...;

fails with [INVALID_ARGUMENT]parse number fail, string: '-----2628', while the same statement with
IF(... , 0, ...) succeeds. sc-content-len is string in Glue, BIGINT in the target table.
Checkout under investigation: 4.0.1-rc02 (791725594de).

The reported string is not in the data. It is five NULL rows' worth of - bytes glued onto the next
real value 2628 — a stale-cursor bug in the strict-mode string→number cast. This is a genuine
upstream Doris defect, still present on master, and in its milder form it corrupts data silently.

---
Root cause (confirmed)

be/src/vec/data_types/serde/data_type_number_serde.cpp:726-756

size_t current_offset = 0;
...
for (size_t i = 0; i < size; ++i) {
    if (null_map && null_map[i]) {
        continue;                                   // <-- skips the cursor update below
    }
    size_t next_offset = (*offsets)[i];
    size_t string_size = next_offset - current_offset;

    StringRef str_ref(&(*chars)[current_offset], string_size);
    if (!try_parse_impl<T>(vec_to[i], str_ref, params)) {
        return Status::InvalidArgument("parse number fail, string: '{}'",
                std::string((char*)&(*chars)[current_offset], string_size));
    }
    current_offset = next_offset;                   // <-- never reached for NULL rows
}

ColumnString stores row i as chars[offsets[i-1] .. offsets[i]). This loop re-derives the row
start from a running current_offset cursor instead of offsets[i-1], and the continue for
rows bypasses the cursor advance. After the first NULL row the cursor is stale, so every subsequent
non-NULL row is read as the concatenation of all skipped NULL rows' bytes plus its own va

The StringRef handed to try_parse_impl is the same wrong slice, so this is not merely a b
message — the parse itself operates on garbage.

Sibling serdes that index per-row via col_str.get_data_at(i) — data_type_datev2_serde.cpp,
data_type_datetimev2_serde.cpp, data_type_date_or_datetime_serde.cpp, data_type_ipv4_serd
data_type_ipv6_serde.cpp, data_type_time_serde.cpp — are all immune. Only the two that hand-roll
the cursor are affected:

┌────────────────────────────────────────────────────────────┬───────────────────────────────────────────┐
│                            File                            │            Function            │               Status               │
├────────────────────────────────────────────────────────────┼───────────────────────────────────────────┤
│ be/src/vec/data_types/serde/data_type_number_serde.cpp:726 │ from_string_strict_mode_batch  │ buggy (all int/float/bool targets) │
├────────────────────────────────────────────────────────────┼───────────────────────────────────────────┤
│ be/src/vec/data_types/serde/data_type_decimal_serde.cpp:74 │ from_string_strict_mode_batch  │ buggy (identical shape)            │
├────────────────────────────────────────────────────────────┼───────────────────────────────────────────┤
│ be/src/vec/data_types/serde/data_type_number_serde.cpp:699 │ from_string_batch (non-strict) │ correct — advances unconditionally │
└────────────────────────────────────────────────────────────┴───────────────────────────────────────────┘

Why NULL rows still carry - bytes

A NULL row in Nullable(String) normally has an empty nested slice, because every null-pro
entry point (insert_default, insert_many_defaults, insert_data(nullptr, ·), insert(Field(Null)))
routes to ColumnStr::insert_default(), which pushes offsets.push_back(chars.size()). When
nested slice is empty, offsets[i] == offsets[i-1], the stale cursor happens to stay correct, and the
bug is invisible. That is why this has survived unnoticed.

IF(cond, NULL, nullable_col) breaks that assumption. be/src/vec/functions/if.cpp:249-256:

if (is_column_nullable(*arg_else.column)) {           // if(cond, null, nullable)
    auto result_column = (*std::move(arg_else_column)).mutate();
    assert_cast<ColumnNullable&>(*result_column)
            .apply_null_map(assert_cast<const ColumnUInt8&>(*arg_cond.column));

ColumnNullable::apply_null_map_impl (column_nullable.cpp:541-554) only ORs the null map —
arr1[i] |= arr2[i] — and never touches the nested column. The rows that just became NULL
their original - bytes in chars, with non-empty offset deltas.

Full chain for the failing query

1. FE: findCommonPrimitiveTypeForCaseWhen (TypeCoercionUtils.java:1949-1958) — "string-like vs all
other type" → common type StringType. So the plan is cast(if(col = '-', cast(null as stri
2. FE: SessionVariable.enableStrictCast() (SessionVariable.java:5603-5614) returns
enableInsertStrict when statementContext.isInsert(). enable_insert_strict defaults to tru
so the cast runs in CastModeType::StrictMode regardless of enable_strict_cast (default false).
3. BE if(): execute_for_null_condition → recurse → execute_for_null_then_else → apply_nul
Result is Nullable(String) whose nested ColumnString still holds - under the new NULLs.
4. BE cast: function_cast.cpp:176-189 prepare_remove_nullable unwraps the nullable and pa
raw null map down. need_replace_null_data_to_default (function_cast.cpp:107-111) returns
false when the source is DataTypeString, so the nested chars are handed over unscrubbed.
5. cast_to_basic_number_common.h:455-463 → from_string_strict_mode_batch(*col_from, *column_to, opts, null_map).
6. Five consecutive - rows are skipped without advancing the cursor; row six is read as -
Status::InvalidArgument → (host)[INVALID_ARGUMENT]… (status.cpp:44) → FE
AbstractInsertExecutor → UserException.getMessage() prefixes errCode = 2, detailMessage =

Why IF(col = "-", 0, col) works

Same common type (StringType), so the plan becomes if(col = '-', '0', col). then is a non
constant, so execute_for_null_then_else does not fire; execute_for_nullable_then_else builds the
nested result from scratch via execute_generic → insert_from per row, producing a compact
correctly-offset ColumnString with 0 where the dashes were. No new NULLs are introduced, the null
map is all-zero, the cursor never desyncs. It is not that 0 is safer than NULL — it is th
row with non-empty bytes ever reaches the cast.

---
⚠️ Silent data corruption (the more serious half)

The error you hit is the lucky outcome. With a single NULL row before a value, the glued
is still parseable:

┌───────────────────────────┬──────────────┬───────────────────────────────────────────────┐
│ NULL run before the value │ Glued string │                    Result
├───────────────────────────┼──────────────┼───────────────────────────────────────────────┤
│ - ×1, then 2628           │ -2628        │ parses as -2628 — silently inserted, no erro
├───────────────────────────┼──────────────┼───────────────────────────────────────────────┤
│ - ×2+, then 2628          │ --2628 …     │ parse fails → visible error
└───────────────────────────┴──────────────┴───────────────────────────────────────────────┘

So a load that "succeeds" on this code path can write negated content lengths. Any table already
loaded with CAST(<expr producing NULLs over non-empty strings> AS <number/decimal>) under
insert should be treated as suspect. Worth a sanity check on any existing data:

SELECT count(*) FROM internal.cloudfront.test WHERE `sc-content-len` < 0;

The same applies to any DECIMAL target via data_type_decimal_serde.cpp.

---
Minimal reproduction (shareable with the Doris team)

No Hive, Glue, S3 or external catalog needed — a plain OLAP table reproduces it. One buck
duplicate key on id keep row order deterministic within the block.

CREATE TABLE test_strict_cast_null_offset (
  id INT,
  s  VARCHAR(16)
)
DUPLICATE KEY(id)
DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1");

INSERT INTO test_strict_cast_null_offset VALUES (1, '-'), (2, '-'), (3, '2628');

SET enable_strict_cast = true;

SELECT id, CAST(IF(s = '-', NULL, s) AS BIGINT) AS v
FROM test_strict_cast_null_offset ORDER BY id;

Expected: 1 → NULL, 2 → NULL, 3 → 2628.
Actual: ERROR 1105 (HY000): … [INVALID_ARGUMENT]parse number fail, string: '--2628'
— the two skipped NULL rows' - bytes are prepended to row 3's value.

Silent wrong-result variant (the more important one for the report) — drop to a single NU
the glued string still parses:

TRUNCATE TABLE test_strict_cast_null_offset;
INSERT INTO test_strict_cast_null_offset VALUES (1, '-'), (2, '2628');

SELECT id, CAST(IF(s = '-', NULL, s) AS BIGINT) AS v
FROM test_strict_cast_null_offset ORDER BY id;

Expected: 1 → NULL, 2 → 2628. Actual: 2 → -2628, no error.

Notes for the report:
- SET enable_strict_cast = true is only needed for a bare SELECT. An INSERT … SELECT take
strict path automatically because SessionVariable.enableStrictCast() returns enable_insert_strict
(default true) for insert statements — which is how this shows up in real loads.
- CAST(… AS DECIMAL(10,2)) reproduces identically via data_type_decimal_serde.cpp.
- CAST(… AS DATE/DATETIME/IPV4) does not reproduce — those serdes index with get_data_at(
- Any expression that marks rows NULL without clearing the nested string works, e.g.
NULLIF(s, '-') in place of the IF. A plain WHERE-filtered column does not, because filter
rebuilds the ColumnString compactly.

---
Security assessment

Not a security vulnerability — a data-integrity bug.

- No memory-safety impact. current_offset is always a previously-read entry of a monotoni
non-decreasing offsets array, so string_size = offsets[i] - current_offset cannot underflow and
the StringRef never leaves the chars buffer. Over-wide read, in-bounds.
- No information disclosure across a trust boundary. The extra bytes belong to adjacent rows of the
same column in the same query — data the caller can already SELECT.
- No privilege escalation, no DoS, no crash, no remote trigger. The party who writes the SQL is the
party whose data is corrupted.

If forced onto the CVSS 3.1 scale: AV:L/AC:H/PR:L/UI:R/S:U/C:N/I:L/A:N ≈ 2.5 (Low), drive
solely by the silent integrity impact. Report as a normal GitHub issue, not to security@apache.org.

---
Version status

Verified from_string_strict_mode_batch in data_type_number_serde.cpp (moved to
be/src/core/data_type_serde/ on master) across the tree:

┌─────────────────────────────────────────────────────┬────────┐
│                         Ref                         │ Status │
├─────────────────────────────────────────────────────┼────────┤
│ 4.0.1-rc02 (this checkout)                          │ buggy  │
├─────────────────────────────────────────────────────┼────────┤
│ 4.0.5-rc01, 4.0.8                                   │ buggy  │
├─────────────────────────────────────────────────────┼────────┤
│ 4.1.0, 4.1.3                                        │ buggy  │
├─────────────────────────────────────────────────────┼────────┤
│ origin/branch-4.0, origin/branch-4.1, origin/master │ buggy  │
└─────────────────────────────────────────────────────┴────────┘

Nothing in 4.0.1-rc02..origin/master touches these two functions except the file move and a
try_parse_impl<T, true> template-arg change. gh searches for existing issues/PRs on
from_string_strict_mode_batch and "parse number fail strict cast null" found nothing.
No upstream fix exists — this needs reporting.

---
Immediate workarounds

Recommended (keeps strict insert, no NULL ever enters the cast — sc-content-len is never -1):

NULLIF(CAST(IF(`sc-content-len` = "-", "-1", `sc-content-len`) AS BIGINT), -1) AS `sc-content-len`

Alternative, if the extra CPU is acceptable — relaxing the cast makes - become NULL naturally and
uses the correct from_string_batch path, but it also weakens error checking for the whole

SET enable_insert_strict = false;
-- then simply: CAST(`sc-content-len` AS BIGINT)

Do not just swap the branches (IF(col <> '-', col, NULL)) — that takes the symmetric
apply_negated_null_map path at if.cpp:282-288 and hits exactly the same bug.

---
Implementation plan — test-first

Scope rule: nothing gets fixed unless a test covers it. Anything in this document without a test in
Phase 1 stays untouched; the out-of-scope list at the end says what that excludes and why
defects qualify — the two serde cursor bugs and the reproducible half of the if() shared-null-map
defect, which T4 pins directly.

Work top to bottom. Do not touch data_type_number_serde.cpp, data_type_decimal_serde.cpp
if.cpp until Phase 2 has shown every new test failing for the right reason.

Phase 0 — get onto master

This checkout is a detached HEAD at 4.0.1-rc02. All work happens on the upstream default branch,
which is master (this repo has no main):

git fetch origin
git switch master

No branch is created and nothing is staged or committed — all changes stay in the working tree until
you decide otherwise.

Verified present on master before starting: both serde cursor bugs, and if.cpp:251-253 /
:282-284 unchanged, with still no ColumnNullable::mutate() override.

Paths differ on master — the file-and-line references in the analysis above are from the 4.0.1
checkout. Translate as:

┌─────────────────────────────────────────────────────────┬───────────────────────────────┐
│                          4.0.1                          │                         master                          │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/src/vec/data_types/serde/data_type_number_serde.cpp  │ be/src/core/data_type_serde/data_type_number_serde.cpp  │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/src/vec/data_types/serde/data_type_decimal_serde.cpp │ be/src/core/data_type_serde/data_type_decimal_serde.cpp │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/src/vec/functions/if.cpp                             │ be/src/exprs/function/if.cpp                            │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/src/vec/columns/column_nullable.{h,cpp}              │ be/src/core/column/column_nullable.{h,cpp}              │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/src/vec/columns/column_string.h                      │ be/src/core/column/column_string.h                      │
├─────────────────────────────────────────────────────────┼───────────────────────────────┤
│ be/test/vec/function/cast/                              │ be/test/exprs/function/cast/                            │
└─────────────────────────────────────────────────────────┴───────────────────────────────┘

Once the upstream PR lands, cherry-pick to branch-4.1 and branch-4.0 — the latter is the
actually running in production here.

Phase 1 — write the tests (no production changes)

Paths below are the master layout (see the Phase 0 table).

T1 — BE unit test, integer strict cast. New file
be/test/exprs/function/cast/cast_to_int_from_string_with_nulls_test.cpp, following the ha
be/test/exprs/function/cast/cast_test.h and the existing cast_to_boolean_test.cpp, which already
drives from_string_strict_mode_batch.

Behaviour to assert: a strict-mode batch cast reads each row's own bytes, regardless of p
NULL rows. Build a ColumnString that holds non-empty text on rows whose null-map bit is set — this
is the state if() produces and the state no existing test constructs:

┌──────┬──────────────────────────────┬───────────────┬──────────────────────────────────────────────┐
│ Case │        Input strings         │   Null map    │                            Expected result                             │
├──────┼──────────────────────────────┼───────────────┼──────────────────────────────────────────────┤
│ T1a  │ ["-", "2628"]                │ [1, 0]        │ OK; [·, 2628] — pins the silent -2628 corruption                       │
├──────┼──────────────────────────────┼───────────────┼──────────────────────────────────────────────┤
│ T1b  │ ["-","-","-","-","-","2628"] │ [1,1,1,1,1,0] │ OK; [·,·,·,·,·, 2628]                                                  │
├──────┼──────────────────────────────┼───────────────┼──────────────────────────────────────────────┤
│ T1c  │ ["-", "2628"]                │ [0, 0]        │ error, message quotes exactly '-'                                      │
├──────┼──────────────────────────────┼───────────────┼──────────────────────────────────────────────┤
│ T1d  │ ["", "2628"]                 │ [1, 0]        │ OK; [·, 2628] — the empty-slice case that passes today, guards the fix │
└──────┴──────────────────────────────┴───────────────┴──────────────────────────────────────────────┘

· = value under a set null-map bit; assert only the status and the non-null rows.

T2 — BE unit test, decimal strict cast. Same four cases against
DataTypeDecimalSerDe<T>::from_string_strict_mode_batch, in
be/test/exprs/function/cast/cast_to_decimal_from_string_with_nulls_test.cpp.

T3 — SQL regression test. New
regression-test/suites/function_p0/cast/test_cast_string_to_number_with_nulls.groovy, alongside
test_cast_to_complex_types_strict.groovy. Use the table and queries from Minimal reproduc
above: both the '--2628' error case and the silent -2628 case, for BIGINT and DECIMAL, under
enable_strict_cast = true and again via INSERT … SELECT with the default enable_insert_st

T4 — BE unit test for the secondary defect. be/test/vec/columns/column_nullable_test.cpp,
as written in the Secondary defect section below. This is the only test that covers if.cpp, so it
alone authorises the if.cpp change.

Phase 2 — confirm red

Build BE and run T1, T2, T4; start a cluster and run T3. Every new case must fail, and th
match the predicted mechanism — T1a/T1b reporting -2628 / '--2628', T4 showing src has acquired
nulls. T1c and T1d must pass already; if they don't, the diagnosis is wrong and this plan
revisiting before any fix.

Phase 3 — fix

F1 → covered by T1, T3. be/src/core/data_type_serde/data_type_number_serde.cpp,
from_string_strict_mode_batch. Delete the current_offset / chars / offsets locals and ind
row:

const auto str_ref = str.get_data_at(i);
if (!try_parse_impl<T>(vec_to[i], str_ref, params)) {
    return Status::InvalidArgument("parse number fail, string: '{}'", str_ref.to_string());
}

ColumnStr::get_data_at (column_string.h:140-144) is
StringRef(&chars[offsets[i-1]], offsets[i] - offsets[i-1]), and offsets[-1] == 0 is guaranteed by
PaddedPODArray's zeroed left pad, so row 0 needs no special case. This matches the alread
date, time and IP serdes.

F2 → covered by T2, T3. be/src/core/data_type_serde/data_type_decimal_serde.cpp,
from_string_strict_mode_batch. Identical change around the CastToDecimal::from_string(...

F3 → covered by T4. be/src/exprs/function/if.cpp:251-253 and the symmetric :282-284. Buil
freshly allocated OR-ed null map instead of mutating through a shallow clone. Prefer this over a deep
ColumnNullable::mutate() override — narrower, and safe to backport.

Both F1 and F2 are pure simplifications: no new helpers, and no behaviour change for the
case that T1d locks down.

Phase 4 — confirm green

Re-run T1–T4; all pass. Then re-run the two existing suites that assert on the parse number fail
text, regression-test/suites/load_p0/tvf/test_tvf_strict_mode_and_filter_ratio.groovy and
test_tvf_error_column.groovy, plus be/test/exprs/function/cast/cast_to_boolean_test.cpp and
be/test/core/column/column_nullable_test.cpp. Finally re-run the real Glue INSERT with th
IF(..., NULL, ...) expression.

Phases 2 and 4 are cheaper on a debug BE: ColumnStr::sanity_check_simple is #ifndef NDEBUG only, so
a debug build makes any offset divergence loud rather than silent.

Out of scope (no test, so no fix)

- ColumnVector::resize not zero-filling (column_vector.h:263), leaving uninitialised valu
under null rows in from_string_strict_mode_batch. Logically masked by the null map; no observable
behaviour to assert against.
- The in-place condition mutation at if.cpp:432-438 (nested_bool_data[i] &= !null_map[i]). Same
const-cast class of defect as F3, but it only touches nested bool values under NULL rows,
unobservable by design — so no test can pin it. Leave it.
- A deep ColumnNullable::mutate() override. The correct long-term fix per cow.h:358-360,
changes every ColumnNullable consumer and T4 does not cover that blast radius. Raise separately.

Upstream

Worth an apache/doris issue + PR against master with cherry-picks to branch-4.1 and branch-4.0.
Include the silent -2628 case in the issue — it reframes this from "confusing error messa
"silent wrong results", which should get it picked up quickly. F3 is a separate defect in a separate
file; raise it as its own issue and PR rather than bundling.

---
Secondary defect — if() writes through a shallow clone (in scope, via T4)

if.cpp:251-256 uses (*std::move(arg_else_column)).mutate(). Because arg_else.column is still
live in the block, COW::shallow_mutate (cow.h:305-311) takes the use_count() > 1 branch a
calls clone() — but ColumnNullable(const ColumnNullable&) = default copies WrappedPtr members,
so the clone shares the original's null-map object. chameleon_ptr::operator*() non-const
bare const_cast (cow.h:342), so apply_null_map writes through into the input column's null map.
cow.h:358-360 explicitly says a class with WrappedPtr members must reimplement mutate() t
mutate; ColumnNullable does not.

Harmless for the failing INSERT (the source column is not otherwise projected), but it means
IF(col = '-', NULL, col) can retroactively NULL out col itself for any other consumer of
block column.

Verification cases

Reuse the same table, and keep the target a string so the primary cast bug stays out of the way:

CREATE TABLE test_if_shared_nullmap (
  id INT,
  s  VARCHAR(16)          -- must be NULLABLE; a NOT NULL column takes a different if() branch
)
DUPLICATE KEY(id)
DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1");

INSERT INTO test_if_shared_nullmap VALUES (1, '-'), (2, '-'), (3, '2628');

S1 — sibling projection (visual). s must be unaffected by what IF does.

SELECT id, s, IF(s = '-', NULL, s) AS v FROM test_if_shared_nullmap ORDER BY id;

Correct: s = -, -, 2628 and v = NULL, NULL, 2628.
Defect present: s also comes back NULL, NULL, 2628.

S2 — same thing as a scalar assertion, easier to put in a regression test:

SELECT count(s) AS cnt_s, count(IF(s = '-', NULL, s)) AS cnt_v FROM test_if_shared_nullma

Correct: cnt_s = 3, cnt_v = 1. Defect present: cnt_s = 1.

S3 — the symmetric branch (if.cpp:282-288, apply_negated_null_map), which must be fixed t

SELECT count(s) AS cnt_s, count(IF(s <> '-', s, NULL)) AS cnt_v FROM test_if_shared_nullm

Correct: cnt_s = 3, cnt_v = 1. Defect present: cnt_s = 1.

S4 — BE unit test (the definitive check). S1–S3 depend on the planner handing if() the sa
ColumnPtr the projection later reads, so a clean result there does not clear the code defect. This
does, and belongs in be/test/core/column/column_nullable_test.cpp. Behaviour to assert: t
mutable handle to a shared nullable column and applying a null map leaves the original column
unchanged.

// build src = Nullable(String) ["-", "-", "2628"], all non-null
ColumnPtr src = ...;
auto cond = ColumnUInt8::create();      // {1, 1, 0}
auto held = src;                        // second reference, as if(...) does
auto mutated = (*std::move(held)).mutate();
assert_cast<ColumnNullable&>(*mutated).apply_null_map(*cond);

EXPECT_FALSE(assert_cast<const ColumnNullable&>(*src).has_null());   // fails today
EXPECT_TRUE(assert_cast<const ColumnNullable&>(*mutated).has_null());

S4 is T4 in the phased plan and is what authorises the F3 change. S1–S3 are diagnostics o
them for the upstream issue, but do not gate the fix on them, since a clean result there reflects plan
shape rather than the absence of the defect.

The in-place condition mutation at if.cpp:432-438 and a deep ColumnNullable::mutate() ove
both listed under Out of scope above — no test can pin the former, and T4 does not cover the blast
radius of the latter.