Spec: TextField in a numeric column makes the whole contentlet unindexable (#37272)

[fabrizzio-dotCMS]

PR 1 of 2 — spec only. Carries spec.md and nothing else. Please review it as a spec, not as code: is this the right problem, scoped right, with measurable criteria? Once approved I branch off this branch and open PR 2 with the implementation. No need to wait for this to merge.

Refs #37272

TL;DR

What's broken: a Text field whose value is stored as a String but whose backing column is numeric (integer1) makes the entire contentlet disappear from the index. Silent — the save works, the UI flags nothing, the reindex shows only a failure count.

Why: ESMappingAPIImpl.loadFields picks the serialization branch by the storage column name instead of by the value. "integer1" starts with integer, so the String reaches DecimalFormat.format(), which accepts only Number and always throws. The per-field catch then logs at WARN and rethrows, aborting the whole document.

Context worth knowing before reviewing:

• The modelling is legitimate and supported — TextField.acceptedDataTypes() includes INTEGER/FLOAT, and dotCMS ships content types shaped this way (htmlpageasset.sortOrder, Vanity URL.order). What decides the failure is the Java class of the value, not the modelling.
• Not a regression. Both defects are verbatim in e8ef584ec9 — initial trunk import, 2012. The ES→OpenSearch work neither introduced nor worsened it; the bulk refresh just pushed enough documents through the same path to expose it.
• ESMappingAPIImpl is the shared mapper for both engines (OSBulkHelper:250 calls the same toMap), so the fix covers ES and OS at once.

Proposed fix

Best-effort conversion at index time. If the value converts, the numeric branch behaves exactly as today. If it does not, emit 0 under the numeric key (consistent with the field's long mapping) with the original text preserved in _dotraw, plus a WARN naming the field and content type. And a failing field no longer kills the document.

Out of scope, with the reasoning written down

• Content-type-save-time validation (the issue's "consider" bullet) — that validation already exists and already says this modelling is legal (FieldFactoryImpl:478). Forbidding the combination means removing INTEGER from TextField.acceptedDataTypes(), which breaks dotCMS's own built-in content types at bootstrap.
• Field.fieldValue() respecting dataType() — the preventive counterpart, and it would also close the ImportStarterUtil route, but it changes every save of every contentlet in the system. Different blast radius, deserves its own cycle.
• Changing the index mapping generator; repairing existing stored values; backfilling lost documents.

Identified risk

ImportStarterUtil writes contentlet JSON verbatim, bypassing the API entirely (:894 → toMutableContentlet, :782 → factory save). Every user-facing path coerces (new editor, legacy editor, Content Import portlet and the new import job all funnel through setContentletProperty → integerStrategy), so the starter import is the most plausible provenance of the observed rows — and it explains why the defect cannot be reproduced through the UI. Named as a known gap; not fixed here.

What to scrutinise

AC-005. htmlpageasset.sortOrder has exactly this field shape, and its zero-padded _dotraw (0000000000000000054.000000000000000000) is what makes lexicographic sorting equal numeric sorting for page ordering. That document must stay byte-identical. A type-based rule instead of a value-based one would have broken page ordering on every installation — that trade-off is recorded under Resolved Decisions → C-1.

🤖 Generated with Claude Code

[claude]

Claude finished @fabrizzio-dotCMS's task in 1m 34s —— View job

---

SDK Compatibility Analysis

• Read docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md
• Reviewed the full PR diff
• Checked against every breaking-change category (G-1/G-2/G-3, R-1, U-1/U-2, H-1)
• Conclusion posted

Result: No SDK-breaking change detected.

This PR is spec-only (per the PR description: "PR 1 of 2 — spec only. Carries spec.md and nothing else."). The single changed file is specs/37272-textfield-numeric-column/spec.md (+384/-0) — a Spec-Kit planning document describing a proposed fix for ESMappingAPIImpl.loadFields (TextField-on-numeric-column indexing failure). No production code is touched.

None of the SDK-relevant surfaces are affected:

• No GraphQL schema/resolver changes (G-1/G-2/G-3 — no graphql.page/graphql.content reachable types or args touched)
• No REST response shape changes (R-1 — no /api/v1/nav, /api/v1/content, /api/v1/page/* changes)
• No UVE postMessage protocol changes (U-1/U-2 — no __DOTCMS_UVE_EVENT__ or DotCMSUVEAction changes)
• No SdkVersionWebInterceptor / X-DotCMS-Version / X-DotCMS-Min-SDK / compareVersions() changes (H-1)

This matches the categories doc's own non-breaking calibration example: "Test-only or documentation-only changes." No comment or label added, per instructions — the implementation (PR 2, once opened) will need this same check applied to the actual code change.

[fabrizzio-dotCMS]

Addressed all four points, plus one that came out of chasing the first one and turned out to be the bigger of the two. Pushed in a95a723. @dario-daza — re-review please, the _dotraw change alters what the fix must emit.

The one that wasn't on the list: _dotraw, not the numeric key, is load-bearing

The review's 🟡 #1 asked whether 0 is the right sentinel under the numeric key. Valid question, but chasing it surfaced that the more consequential key is the other one, and that the old AC-007 was wrong about it.

Chain, all verified on the branch:

1. loadFields writes the padded string to keyNameText = <field>_text — ESMappingAPIImpl.java:1009, :1118.
2. That entry is lowercased (:590) and renamed _text → _dotraw (:598).
3. By default (CREATE_TEXT_INDEX_FIELD_FOR_NON_TEXT_FIELDS=false) the _text key is dropped (:607-610) — so _dotraw is the only survivor of the pair.
4. *_dotraw is mapped keyword (es-content-mapping.json, template_1) — lexicographic, not numeric.
5. Every sort targets it: addBuilderSort appends _dotraw to whatever field name the caller passes — ContentFactoryIndexOperationsES.java:412-413, and the OpenSearch counterpart.

So the zero-padding is the only reason ordering by a numeric field works at all. The old AC-007 put the raw text under _dotraw ("so the real value stays visible and searchable") — a "N/A" then sorts after every padded digit string ('N' = 0x4E > '9' = 0x39) and silently reorders any listing sorted by that field, with nothing but the WARN to reveal it.

Worth noting the proposed alternative in the review — "omit the numeric key entirely and keep only _dotraw" — preserves this, since it leaves the raw text in the key that actually sorts.

AC-007 is now the invariant: <field>_dotraw is either the 19.18 zero-padded string or absent, never raw text, never an unpadded number — asserted end-to-end by sorting a result set asc and desc with an unconvertible document present. This one is forced by the mapping, not a design choice.

🟡 #1 — 0 vs omit: decided, omit

Split out as AC-008 and recorded as C-3 with the rejected alternative. Omit rather than 0: a 0 is indistinguishable from a genuine 0 in range queries, sorts and aggregations — field:[0 TO 10] would match a document whose value is really "N/A", and only the WARN reveals it. Omission leaves the field honestly unset for that one document, and the engines' defined missing-key behavior applies (sorts last). A false in-range hit is a correctness bug; an absent field is not.

Flagging this as the one genuinely overturnable call in the change — the _dotraw half is a correctness fix, this half is judgment. The plan may reverse it if the reason gets recorded.

You also read the Data-considerations sentence as "must not write a non-numeric string under the numeric key". Fair reading, but it literally said "must not write the numeric key" — ambiguous between that and "don't write it at all". Disambiguated, since C-3 now depends on which one it is.

🟡 #2 — float path: confirmed, and worse than described

Right, and there's a second reason it matters: NumberUtil has no toFloat/toDouble at all — only toInt, toLong, toBoolean, asInt. So there's nothing to lean on for the float case, and NumberUtil.pad(Number) takes a Number, so it can't be used on a value that didn't convert either.

Also corrected a related claim the spec had wrong: the generated mapping is long for an integer% column but double for a float% one — ESMappingUtilHelper.java:404-406, DataTypes.INTEGER → long, DataTypes.FLOAT → double. The spec said long flatly. The unit matrix now requires both columns with a convertible ("54", "54.3") and an unconvertible value.

🟡 #3 — wording: taken, plus a trap it creates

"byte-identical" → "identical emitted map entries" in AC-005, AC-006 and AC-009. But asserting at the Map level — which is the right layer — makes runtime class matter: Integer 54 and Long 54 are not equals. So the helper must return the same class the native path produces, or the Red test passes or fails for the wrong reason. Called out in AC-009 so PR 2 doesn't step in it.

🟢 #4 — no-WARN on the happy path: added

AC-010: a natively-stored number (htmlpageasset.sortOrder) and a convertible String both produce no WARN at all.

---

Verified every line citation in the review before acting on it — :1113-1116, :1118, :996, :1142-1145, TextField.java:33, the Date-branch asymmetry. All exact.

[ihoffmann-dot]

Review:

I think the spec looks solid, root cause is right, and the sortOrder call in C-1 is a genuinely good catch. Three things I'd want tightened before I approve:

• First, there's a contradiction. Regression Risk says the text-degradation path must not write the numeric key. AC-007 says it writes 0 under the numeric key. Those can't both be true, I think we should pick one and say it once.

• Second, and this is the real one: I'm not sure 0 is the right sentinel. Two side effects aren't in the spec. Range queries, a search for field:[0 TO 10] starts matching content whose actual value is "abc". That's a silent false positive, which is the same class of invisible failure we're fixing. And unique fields: the sha256 is computed from whatever lands under the numeric key, so two different unconvertible values both become 0, hash identically, and the unique-field validator reports a false duplicate. AC-006 only covers correctly-modelled fields, so nothing catches that today. The alternative is to just omit the numeric key and keep the text in _dotraw. Elasticsearch doesn't require every document to carry every mapped field, so there's no mapper exception either way. If we keep 0, I'd like C-1 to say why, plus an AC for the unique-field case.

• Third, the conversion rule isn't column-aware. The spec says "follow the NumberUtil.toInt / toLong idiom", but doesn't say integer% converts to a whole number and float% to a decimal. Run toLong on a float column and a legitimate 54.7 silently truncates to 54, which breaks AC-006.

One and two are the blockers for me. Three is just wording, but they're the kind of wording that turns into a bug.

FYI @fabrizzio-dotCMS