block / block/elasticgraph

`returnable: false` silently wipes indexed values on multi-source (`sourced_from`) and derived indexed types

Open
#1,317 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Ruby
Stars
88
Forks
36
Avg merge
5d 10h
Merged PRs (30d)
18

Description

#1108 added `returnable: false`, which (among other things) emits a `_source.excludes` entry for the field so its value isn't stored in the compressed `_source` blob. That part of the feature is unsafe on any index whose documents are updated with partial data — most notably types with `sourced_from` fields — and there's currently no validation preventing the combination.

## The mechanism

Elasticsearch/OpenSearch scripted updates don't patch a document in place. The `_update` API reads the stored `_source`, hands it to the script as `ctx._source`, and then **re-indexes the entire document from the merged result**. A field covered by `_source.excludes` is indexed (doc values + inverted index) but never stored — so on the next scripted update, its value is simply absent from `ctx._source`. Unless the incoming event re-supplies it, the re-indexed document no longer contains the field at all: its doc values and inverted-index entries are silently dropped.

The ES docs call this out directly — fields pruned from `_source` are unavailable to APIs that rely on it, including `update`, `update_by_query`, and `reindex` ([`_source` field docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-source-field.html#include-exclude)).

Note the failure is at **(re)index time**, not retrieval time. It corrupts filtering, sorting, grouping, and aggregations — exactly the features `returnable: false` promises to keep working — and no retrieval-side change can compensate. There's also no error: queries just quietly stop matching documents after an unrelated source's update touches them.

## Why single-source types are safe

ElasticGraph's `index_data` update script applies each event via `doc.putAll(params.topLevelFields)` against `ctx._source`. For a single-source type, every `__self` event carries the *complete* document payload, so excluded fields are re-supplied on every update and nothing is lost.

## Where it breaks

1. **Types with `sourced_from` fields.** An event arriving via a relationship (e.g. the `Country` side of the documented `City.currency` example) supplies only that relationship's fields. Every `returnable: false` field owned by `__self` is missing from `ctx._source` and gets wiped from the re-indexed document. The reverse direction is broken too: a later `__self` event re-supplies self fields but not top-level `sourced_from` fields, so `returnable: false` on a `sourced_from` field is equally unsafe. (Nested `sourced_from` fields might incidentally survive via the `__nested_sourced_data` re-apply buffer, since the buffer's path isn't excluded — but that's an accident of representation, not a guarantee.)

2. **Derived indexed types (`derive_indexed_type_fields` destinations).** Derivation scripts read the current accumulated value out of `ctx._source` (e.g. append-only sets) and rebuild it. With the field stripped from `_source`, every event resets the accumulated state to just that event's contribution.

## Repro sketch

```ruby
schema.object_type "City" do |t|
t.field "id", "ID"
t.field "population", "Int", returnable: false # filterable, not returned
t.relates_to_one "capitalOf", "Country", via: "capitalCityId", dir: :in
t.field "currency", "String" do |f|
f.sourced_from "capitalOf", "currency"
end
t.index "cities"
end
```

1. Ingest a `City` event with `population: 100`. The doc is indexed; `population` is filterable but absent from stored `_source`.
2. Ingest a `Country` event that updates that city's `currency`. The update script reads `ctx._source` (no `population`), merges the currency, and the doc is re-indexed without `population`.
3. `filter: {population: {gt: 0}}` no longer matches the city; sorting treats it as missing. No error is raised anywhere.

## Proposed fix: make "single-source only" a hard validation

Raise `Errors::SchemaError` at schema-definition time whenever a `_source.excludes` entry would be emitted for an index whose documents can be updated by partial-data events:

- the indexed type's `current_sources` contains anything besides `__self` (it has `sourced_from` fields — directly or on an embedded object type), **or**
- the index has `has_had_multiple_sources!` set (documents built from multiple sources may still exist even if the `sourced_from` fields were since removed), **or**
- the type is the destination of another type's `derive_indexed_type_fields`.

Implementation notes:

- The excludes list is computed in `HasIndices#mappings` via `source_excludes_paths` (`elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb`). The first two conditions are locally detectable there (`current_sources`, `index_def.has_had_multiple_sources_flag`).
- The derived-type condition needs schema-wide knowledge (derivations are declared on the *source* type), so the natural hook is `Results#generate_datastore_config` (`results.rb`), which already computes `derived_indexing_type_names`. Simplest approach: run the whole validation there, iterating indexed types where `source_excludes_paths.any?`, rather than splitting it across two sites.
- The error message should explain the wipe mechanism briefly and suggest the two ways out: drop `returnable: false` from the field (or accept `highlightable: true`, which already suppresses the exclude), or keep the type single-source.
- Test coverage: schema-definition unit specs asserting the error for (a) `returnable: false` alongside a `sourced_from` field, (b) `returnable: false` on a `sourced_from` field itself, (c) `returnable: false` on a derived indexed type, (d) `has_had_multiple_sources!` + `returnable: false`; plus the existing single-source behavior staying green.
- Independently worth adding: an integration test on a `sourced_from` fixture demonstrating the wipe (filter stops matching after the related-type event) — to confirm the bug against a real datastore before the validation lands, and to document why the validation exists.

This is deliberately a validation, not a workaround — e.g. having other-source events echo back stored values can't be made safe under out-of-order event processing, and `stored` fields or runtime re-derivation don't restore doc values. If a storage-optimized multi-source story is wanted later, it needs a design of its own.

Contributor guide

Open the contributing guide

Research direction

Start in elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb, especially HasIndices#mappings and source_excludes_paths, then follow Results#generate_datastore_config in results.rb. Add schema-definition coverage for sourced_from, derived indexed types, and has_had_multiple_sources!, while keeping single-source behavior green. Confirm the validation explains the source-wipe risk and add the sourced_from integration regression test if feasible.

Written by the indexing model from the issue text.

Assessment

Tech stack
elasticsearch, ruby
Domain
backend, databases, search
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.