Conversation
Introduce a shared, reusable multi_value mapping parameter in core so field arity — whether a field holds a single value or an array — is a first-class mapping concept rather than being defined inside a specific data-format plugin. This is the primitive requested in opensearch-project#16420, useful to any consumer that projects a fixed schema (columnar formats, SQL/PPL, join). Note on naming: this captures arity, which is distinct from the number of distinct values a field has and unrelated to the cardinality aggregation; hence multi_value rather than "cardinality". - Parameter.multiValueParam() factory on ParametrizedFieldMapper.Builder, so any array-capable field type adopts it in one line and stamping is consistent. Defaults to false; not updateable (arity is fixed once data is written). - MappedFieldType carries the arity via isMultiValued()/setMultiValued(), with FilterFieldType delegating. - KeywordFieldMapper wires the shared factory and stamps the field type. Storage/query consumers (e.g. the Parquet LIST encoding) read the flag back via MappedFieldType#isMultiValued() and are added separately. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
Add a `multi_value: true` mapping parameter to keyword fields so a composite parquet+lucene index stores them as an Arrow/Parquet LIST<element> column instead of a flat column, preserving order and duplicates through the full indexing lifecycle. Write path: KeywordParquetField declares list support and writes each value into the ListVector (startNewValue/setSafe/endValue); the Arrow schema builder emits a LIST field when the mapper's field type is multi-valued; ParquetDocumentInput accumulates repeated values per field rather than deduplicating them; _source is reconstructed from the LIST column on the get-by-id path. Three latent defects that only nested columns expose are fixed: VSRManager passed the schema field through so list children survive schema reconcile; per-column Parquet encoding/compression settings resolve to the list leaf path; and encoding validation checks the element type rather than the LIST wrapper. Index creation rejects a multi_value field used as index.sort.field. Covered by a 9-test durability IT (refresh/flush/force-merge, replication, node-restart recovery, concurrent indexing during peer recovery, cross-format partial failure, bulk mixed valid/invalid, null elements dropped, 50k-element array, dynamic mapping addition), unit tests, and 4 Rust merge tests. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com> # Conflicts: # server/src/main/java/org/opensearch/index/mapper/KeywordFieldMapper.java
An explicit empty array (`"field": []`) on a `multi_value` keyword field was reconstructed as a missing field: the document parser's element loop never fires for an empty array, so no value reached the document input and the LIST column cell was written null. Reconstructed _source then could not tell `[]` apart from an absent field — a standalone reader (datafusion-cli) sees the cell as NULL rather than an empty list. Register an empty-list entry for an empty array, strictly gated to the pluggable data format and multi_value leaves so stock indexing is unchanged. It flows through FieldValuePair.emptyMultiValued into the writer's existing zero-length-list branch, so the cell is written empty-but-non-null and reads back as []. Tighten the durability IT to require every indexed array — including the empty d3 fixture — to reconstruct as a present non-null list, and add a ParquetDocumentInput unit test for the empty-array path. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
Address review feedback on the multi-value indexing path: - Key the per-document field accumulator by field name instead of MappedFieldType instance identity. Identity is sound today (the parser hands back one cached field-type instance per array element) but would silently degrade to last-value-wins if a wrapper were ever returned per element; name keying is robust to that. Field names are unique per logical field within a document, including the _ignored_source.* derived-source companion. - Document that a multi-valued FieldValuePair is mutable and must only be read after the document is finalized; the sole consumer reads through getFinalInput() once parsing completes. - Note in registerEmptyMultiValueArray that every scalar-leaf array route funnels through parseNonDynamicArray, and that a future multi_value type with parsesArrayValue()==true would need equivalent empty-array handling. Tests: pin absent-vs-empty as distinct on-disk states (testAbsentFieldIsDistinctFromEmptyArray) and name-keyed accumulation across distinct same-named instances (testMultiValueAccumulationKeyedByNameNotInstanceIdentity). Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
A flat_object field in a composite (pluggable data format) index is now stored as a single Arrow MAP<utf8, utf8> column instead of being silently skipped, making OTel-style attribute maps (ResourceAttributes, LogAttributes, ...) indexable in the Parquet primary format. - FlatObjectParquetField builds MAP(entries: STRUCT<key, value>) via a new ParquetField.getChildren() hook and writes one map cell per document, preserving document order and duplicate keys; an absent field stays a null cell while an explicit empty object becomes a zero-entry non-null map. - FlatObjectFieldMapper wires the previously-dead pluggable-format parse path: the whole object is handed over in one addField call as ordered (relative path, value) entries. It declares FULL_TEXT_SEARCH capability, and accepts derived source only under a pluggable data format (the columnar store owns _source reconstruction) while still rejecting it for plain Lucene indices. - ParquetField.createField routes MapVector to addToGroup rather than the LIST writer (MapVector extends ListVector). - The native writer's per-column settings resolver now derives every parquet leaf of a nested column (a MAP has key and value leaves), so encoding/compression/bloom settings and type-level defaults reach nested columns; a test pins the derived paths against what arrow-rs actually writes. - Index creation rejects nested (MAP) columns in index.sort.field up front instead of failing at first merge. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
…iveSource FlatObjectMapColumnIT indexes the OpenTelemetry-logs mapping (three flat_object attribute maps alongside date_nanos/byte/text/keyword) on a composite parquet+lucene index and covers the write path end to end: mapping acceptance, documents whose attribute key sets differ per doc, the empty/absent/duplicate-key/null and deeply-nested leaf shapes, durability across refresh, flush and force-merge (the native merge must resolve this column to two leaves rather than one), and a flat_object field added by a mapping update on a live index. Negative tests pin the boundaries: flat_object cannot be multi_value (one map cell already holds many entries) and cannot be an index.sort.field (a nested column has no single sort key). deriveSource for flat_object now omits the field instead of throwing. The field's leaves live only in the primary format's column, so there is nothing in the Lucene reader to rebuild the object from. Throwing was reachable through TranslogLeafReader when index.derived_source.translog.enabled is set, where it would have failed realtime GET for the whole document rather than just this field. testAttributesAreNotYetReturnedInSource asserts the resulting gap so it stays visible and must be inverted when the MAP read path lands. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
PR Reviewer Guide 🔍(Review updated until commit 375fe95)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 375fe95 Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit 6e619bc
Suggestions up to commit 9bdbcc5
Suggestions up to commit 97f31af
Suggestions up to commit 1c9e4d2
Suggestions up to commit 6c3ec12
|
Indexes the shared 100-document otel_logs corpus through the REST API on a fully assembled node with the analytics, composite-engine and parquet-data-format plugins installed, using the OTel/Textbench mapping shape: resource, log and instrumentationScope typed flat_object instead of explicit object trees. This covers what only a real node exercises — plugin wiring and capability assignment across the composite primary/secondary formats, the bulk path, and flush plus force-merge of MAP columns through the native writer. Verified to fail without the flat_object support (searchCapability is not supported for field: log of type: flat_object) and to pass with it. The corpus is a good stress for a map column: resource alone carries nine sub-trees across five services, so key sets differ per document. A second test admits the heterogeneous shapes a real pipeline emits — empty bag, omitted bag, array leaf, explicit null, and previously unseen deep keys. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 6c3ec12 |
…ngines Same mapping, same document, two engines: the internal (Lucene) engine indexes a flat_object into three Lucene fields (the parent path-parts field, _value and _valueAndPath), while the composite engine indexes none of them and keeps the data only in the Parquet MAP column. The cause is capability assignment. flat_object requests FULL_TEXT_SEARCH and COLUMNAR_STORAGE; the parquet primary is offered capabilities first and FlatObjectParquetField claims both, so the lucene secondary gets an empty set and LuceneDocumentInput#addField returns without indexing. keyword and text behave differently on purpose: their parquet implementations do not claim FULL_TEXT_SEARCH, so Lucene claims it and they stay searchable on a composite index — asserted here so the distinction is not mistaken for an accident. Consequence, also asserted: a term query on a flat_object leaf matches on the internal engine, and on the composite engine the field is durable but served by neither format — parquet claims the capability without a read path, Lucene holds no terms. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 1c9e4d2 |
|
❌ Gradle check result for 1c9e4d2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
FlatObjectParquetField claimed FULL_TEXT_SEARCH as well as COLUMNAR_STORAGE, which starved the lucene secondary of the field: the primary is offered capabilities first, so Lucene got an empty set and indexed nothing for it. The field was durable in the Parquet MAP column but searchable by neither format, since the columnar read path cannot project a MAP yet. Parquet now claims only COLUMNAR_STORAGE, matching keyword and text whose parquet fields deliberately leave the inverted index to Lucene. LuceneDataFormat declares flat_object (FULL_TEXT_SEARCH, COLUMNAR_STORAGE) and LuceneFieldFactoryRegistry gains a flat_object factory that expands the mapper's leaf entry list into the same three fields the non-pluggable path writes: _value, _valueAndPath and the parent path-part terms. Terms only — doc values stay with the primary. FlatObjectEngineParityIT asserts the Lucene FieldInfos now match between the composite and internal engines, and that flat_object is exactly as searchable as keyword on a composite index. Transport _search is refused for every field type there (IndexShard will not apply it to a DataFormatAwareEngine), so that limitation is asserted as engine-wide rather than mistaken for a flat_object gap. A dotted-path query through the REST path still fails in the DSL query executor, which resolves field names against the columnar row type where a flat_object sub-path is not a column; asserted in OtelFlattenedAttributesIndexingIT so it flips when routing lands. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 97f31af |
The class javadoc still described the pre-fix state (composite indexing nothing in Lucene for a flat_object) after the capability split was corrected, contradicting the assertions below it. Replaced with the verified layout: both engines index the same three Lucene fields, doc values stay with the primary format on the composite engine, and the Parquet MAP column is the columnar copy alongside. Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 9bdbcc5 |
|
Won't map require each key to have the same value type? Given opensearch flat object supports fluid schema across keys within the flat object hierarchy, |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #22741 +/- ##
============================================
+ Coverage 71.43% 71.55% +0.12%
+ Complexity 76995 76986 -9
============================================
Files 6156 6139 -17
Lines 358470 358092 -378
Branches 52247 52225 -22
============================================
+ Hits 256063 256248 +185
+ Misses 82067 81480 -587
- Partials 20340 20364 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
TextBench's v3 storage-optimisation mapping sets index:false on the
flattened attribute fields, which OpenSearch rejected outright: the
flat_object TypeParser ignored the mapping node entirely, so
checkNoRemainingFields turned every parameter into a mapping error. The
only valid flat_object mapping was {"type":"flat_object"}.
flat_object now declares its parameters the way every other field type
does. DynamicKeyFieldMapper moves from FieldMapper to
ParametrizedFieldMapper (flat_object is its only subclass), so parsing,
serialization, includeDefaults and update-conflict detection come from
the framework instead of being hand-written. The mapper keeps its own
Lucene FieldType, as KeywordFieldMapper does, because
ParametrizedFieldMapper always passes a fresh one down; a second frozen
FIELD_TYPE_NOT_INDEXED carries IndexOptions.NONE. The empty mergeOptions
override is gone — that method is final on ParametrizedFieldMapper, and
merge is now driven by the declared parameters, so index finally gets a
real conflict check instead of silently succeeding.
index:false is accepted only when index.pluggable.dataformat.enabled is
set, to keep the blast radius off plain Lucene indices. There the field
would merely fall back to the doc-values path, which is a behaviour
change for existing mappings with no upside; under a columnar primary it
means something concrete, since requestedCapabilities() stops asking for
FULL_TEXT_SEARCH and the field drops out of the Lucene secondary
altogether.
The parameter's read-back derives from fieldType().isSearchable(),
because FlatObjectFieldType takes its searchability from the _value
sub-field rather than storing a flag, so the round trip through
serialization is asserted explicitly.
Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 6e619bc |
|
❌ Gradle check result for 6e619bc: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
On a composite index _source is not stored: DataFormatAwareEngine.getById
falls through to the DocumentLookupProvider, which reads the parquet row
and rebuilds the document through ArrowValues.toSourceMap. That method
had no MAP branch — only a guard keeping maps out of the LIST branch — so
a flat_object column fell through to the scalar switch, returned null,
and toSourceMap dropped it. The attribute bag was silently absent from
every get-by-id response while being perfectly durable on disk.
toSourceValue now converts a MapVector to a JSON object, mirroring the
handling toJavaValue already had. Keys are emitted verbatim: a leaf
written as {"k8s":{"pod":"x"}} was already flattened to the key
k8s.pod on the way in, so it reads back as {"k8s.pod":"x"}. That is
flat_object's own model — its Lucene _valueAndPath terms use the same
dotted form — and it avoids inventing an un-flattening convention for an
ambiguity that cannot be resolved after the fact.
A Parquet MAP is a repeated group, so a key legitimately repeats when the
source had an array; repeats group back into a list rather than
overwriting, so {"tag":["a","b"]} round-trips. An empty object stays
an empty object rather than vanishing.
Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
|
Persistent review updated to latest commit 375fe95 |
|
❌ Gradle check result for 375fe95: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
2 participants
Description
A
flat_objectfield on a composite parquet+lucene index is now stored as a single Arrow/ParquetMAP<utf8, utf8>column instead of being silently dropped. This makes OpenTelemetry-style attribute bags indexable in the Parquet primary format:Each document contributes one map cell holding however many leaves its object had, so a per-document key set is representable in a column whose type is fixed for the whole file. Before this change
ArrowSchemaBuilderskipped the unregistered type and the field produced no column at all — the values were accepted and then lost.This PR is indexing-side only: write path, storage encoding, schema, and durability. Projecting or filtering map keys is deliberately excluded and needs the columnar read path; see Known gap below.
Why MAP rather than parallel LIST columns
MAPis a repeatedkey/valuegroup that shares repetition levels, so entries stay associated and duplicate keys survive. Flattening into independent parallelLISTcolumns would need positional correlation, which is a writer contract rather than a schema guarantee — andDocumentInput.addField(fieldType, value)carries no element ordinal, so ragged objects would silently misalign. Sub-columns per key were also rejected: the key set is per-document and unbounded, while a Parquet schema freezes on first flush.Write path
FlatObjectParquetFieldbuildsArrowType.Map(false)with a non-nullableentries: STRUCT<key, value>child, via a new overridableParquetField#getChildren()hook.getArrowType()stays primitive-only for every other type.startNewValue/endValue, and for each entryentriesVector.setIndexDefined(i)(the entries struct is non-nullable per the Arrow MAP spec — without this the child values read back null) plus key/valuesetSafe."attrs": {}is a zero-entry non-null cell.ParquetField#createFieldroutesMapVectortoaddToGrouprather than the LIST writer.MapVector extends ListVector, so the guard is explicitlyinstanceof ListVector && !(instanceof MapVector)— without it a map would be written through the list protocol.FlatObjectFieldMapperwires the previously-deadcreatePathFieldsForPluggableFormatinto a realparseCreateFieldForPluggableFormat, handing the whole object over in oneaddFieldcall as an orderedList<Map.Entry>of(relative path, value). Keys are relative to the field (http.status, since the column name already carries the prefix); values are stringified, matching flat_object's Lucene representation. Duplicate keys are preserved, so{"a": [1, 2]}yields twoaentries.flat_objectis single-arity and rejectsmulti_value: true: one map cell already holds many entries, andLIST<MAP>is not built.The actual blocker was derived source, not the column
Enabling
index.pluggable.dataformat.enabledalso enables derived source, and mapping creation callscanDeriveSource()on every field.flat_objectcannot rebuild its nested object from the flattened_valueAndPathdoc values through the generic fetcher framework, so it failed there — meaning the whole mapping was rejected before any column work mattered.FlatObjectFieldTypenow declaressearchCapability()(previouslyrequestedCapabilities()threw for any searchable flat_object field in a pluggable index), andcanDeriveSource()passes only when a pluggable data format is configured, where the columnar store owns_source. A plain Lucenederived_sourceindex still rejects it, unchanged.deriveSourceomits the field rather than throwing. The leaves live only in the primary format's column —LuceneDocumentInputstrips doc values and stored fields for a field the primary format owns — so there is nothing in the Lucene reader to rebuild from. Throwing was reachable throughTranslogLeafReaderwhenindex.derived_source.translog.enabledis set, where it would have failed realtime GET for the whole document rather than just this field.Native writer: per-leaf settings resolution
A nested column has more than one parquet leaf, so
column_path_for/effective_type(LIST-only, single leaf) are replaced byleaves_for/collect_leaves, which walk LIST, MAP and STRUCT and return every leaf with its own type:nnLIST<element>nn.list.elementMAP<k,v>nn.entries.key,n.entries.valueField-level and type-level encoding/compression/bloom settings now apply to each leaf independently, and the type key is derived from the leaf type — so a MAP's
utf8leaves pick up theutf8defaults instead of being keyed off theDebugstring of the wholeMaptype and matching nothing.nested_leaf_paths_match_arrow_rs_writerpins the derived paths against whatArrowSchemaConverteractually produces, so a change in arrow-rs's nested naming fails loudly instead of silently turning every nested per-column setting back into a no-op.Validation
index.sort.fieldnow rejects a nested column at creation time. The native k-way merge can only build a sort key from a primitive leaf and would otherwise fail on the first merge — long after the index started accepting writes.Testing
FlatObjectMapColumnIT(new, 8 tests) indexes the OTel-logs mapping on a composite parquet+lucene index: mapping acceptance, documents whose attribute key sets differ per document, the empty / absent / duplicate-key / null-value / deeply-nested leaf shapes, durability across refresh → flush → force-merge (the native merge must resolve this column to two leaves rather than one), aflat_objectadded by a mapping update on a live index, and the two negative boundaries.VSRManagerTests— end-to-end MAP column through the C Data Interface export and the native arrow-rs writer, including the absent and empty cases.FlatObjectParquetFieldTests— Arrow MAP shape and nullability per the spec, order and duplicate-key preservation, null value inside an entry, and rejection ofmulti_value.FlatObjectFieldMapperTests— the pluggable parse path emits exactly oneaddFieldcall carrying every leaf; a null object is skipped.CompositeFieldCapabilityIT.testFlatObjectFieldUnsupportedbecomestestFlatObjectFieldSupported.Known gap (deliberate, asserted)
Attribute values are written and durable but not yet returned in
_source, because derived source is rebuilt from the Lucene secondary, which intentionally holds nothing for a parquet-owned field.testAttributesAreNotYetReturnedInSourceasserts this so the state is visible rather than a silent surprise, and must be inverted when the read path lands. Reading a MAP column will also need the DataFusion page-index resolver to map nested columns to their real leaves — the same defect fixed for LIST columns in #22685.Related Issues
Part of the composite/Parquet data format effort. Stacked on #22703 and #22713.
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.