Skip to content

Detect when copying to a non existent field #127098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Detect when copying to a non existent field #127098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
765e633
We created a shared data structure (e.g., a map of supported evaluato…
BinhMike Apr 20, 2025
a5c791f
This commit addresses the problem where copy_to references could sile…
BinhMike Apr 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,83 @@ public void validate(IndexSettings settings, boolean checkLimits) {
this.mappingLookup.checkLimits(settings);
}
}

public void validate(IndexSettings settings, boolean checkLimits) {
this.mapping().validate(this.mappingLookup);

// Check required routing for partitioned index
if (settings.getIndexMetadata().isRoutingPartitionedIndex()) {
if (routingFieldMapper().required() == false) {
throw new IllegalArgumentException(
"mapping type [" + type() + "] must have routing required for partitioned index ["
+ settings.getIndex().getName() + "]"
);
}
}

// Validate index mode compatibility
settings.getMode().validateMapping(mappingLookup);

// Check source loader compatibility
try {
mappingLookup.newSourceLoader(null, mapperMetrics.sourceFieldMetrics());
} catch (IllegalArgumentException e) {
mapperMetrics.sourceFieldMetrics().recordSyntheticSourceIncompatibleMapping();
throw e;
}

// Validate nested index sort compatibility
if (settings.getIndexSortConfig().hasIndexSort() && mappers().nestedLookup() != NestedLookup.EMPTY) {
if (indexVersion.before(IndexVersions.INDEX_SORTING_ON_NESTED)) {
throw new IllegalArgumentException("cannot have nested fields when index sort is activated");
}
for (String field : settings.getValue(IndexSortConfig.INDEX_SORT_FIELD_SETTING)) {
NestedObjectMapper nestedMapper = mappers().nestedLookup().getNestedMappers().get(field);
String nestedParent = nestedMapper != null ? nestedMapper.fullPath() : mappers().nestedLookup().getNestedParent(field);
if (nestedParent != null) {
throw new IllegalArgumentException(
"cannot apply index sort to field [" + field + "] under nested object [" + nestedParent + "]"
);
}
}
}

// Validate routing paths
List<String> routingPaths = settings.getIndexMetadata().getRoutingPaths();
for (String path : routingPaths) {
if (settings.getMode() == IndexMode.TIME_SERIES) {
for (String match : mappingLookup.getMatchingFieldNames(path)) {
mappingLookup.getFieldType(match).validateMatchedRoutingPath(path);
}
}
for (String objectName : mappingLookup.objectMappers().keySet()) {
if (path.equals(objectName)) {
throw new IllegalArgumentException(
"All fields that match routing_path must be flattened fields. [" + objectName + "] was [object]."
);
}
}
}

// -------- Begin fix for issue #112812 --------
// If dynamic mapping is disabled, validate copy_to targets exist in mappings
if (settings.isDynamicMappingEnabled() == false) {
for (FieldMapper mapper : mapping().getRoot().mappers()) {
for (String copyToTarget : mapper.copyTo().copyToFields()) {
if (mappingLookup.getFieldType(copyToTarget) == null) {
throw new IllegalArgumentException(
"Field [" + mapper.name() + "] specifies copy_to for a non-existent field [" + copyToTarget + "] " +
"but dynamic mappings are disabled"
);
}
}
}
}
// -------- End fix for issue #112812 --------

// Final check of mapping limits
if (checkLimits) {
this.mappingLookup.checkLimits(settings);
}
}
}
224 changes: 224 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,230 @@
import static org.hamcrest.Matchers.nullValue;

public class MapperServiceTests extends MapperServiceTestCase {
public void testCopyToInvalidTargetWithDynamicFalse() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": false,
"properties": {
"test_field": {
"type": "text",
"copy_to": "missing_field"
}
}
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTrue() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": true,
"properties": {
"test_field": {
"type": "text",
"copy_to": "missing_field"
}
}
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplate() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplateAndDynamicFalse() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": false,
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplateAndDynamicTrue() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": true,
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplateAndDynamicTrueAndDynamicFalse() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": true,
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
],
"_doc": {
"dynamic": false
}
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplateAndDynamicTrueAndDynamicFalseAndDynamicTemplates() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": true,
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
],
"_doc": {
"dynamic": false,
"dynamic_templates": [
{
"template2": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
}
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testCopyToInvalidTargetWithDynamicTemplateAndDynamicTrueAndDynamicFalseAndDynamicTemplatesAndDynamic() {
Exception e = expectThrows(MapperParsingException.class, () ->
createIndex("test-index", Settings.EMPTY, """
{
"mappings": {
"dynamic": true,
"dynamic_templates": [
{
"template1": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
],
"_doc": {
"dynamic": false,
"dynamic_templates": [
{
"template2": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
},
"_doc2": {
"dynamic_templates": [
{
"template3": {
"match": "*",
"mapping": {
"type": "text",
"copy_to": "missing_field"
}
}
}
]
}
}
}
""")
);
assertThat(e.getMessage(), containsString("copy_to referencing non-existent field"));
}

public void testPreflightUpdateDoesNotChangeMapping() throws Throwable {
final MapperService mapperService = createMapperService(mapping(b -> {}));
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,15 @@ public List<ShardContext> shardContexts() {
}
});
}

for (ExpressionMapper em : MAPPERS) {
if (em.typeToken.isInstance(exp)) {
return em.map(foldCtx, exp, layout, shardContexts);
}
}
throw new QlIllegalArgumentException("Unsupported expression [{}]", exp);

// Corrected error: use String.format or plain concatenation
throw new QlIllegalArgumentException("Unsupported expression [" + exp + "]");
}

static class BooleanLogic extends ExpressionMapper<BinaryLogic> {
Expand Down
26 changes: 16 additions & 10 deletions .../esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Bucket.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,29 +264,35 @@ public boolean foldable() {

@Override
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
if (field.dataType() == DataType.DATETIME || field.dataType() == DataType.DATE_NANOS) {
DataType type = field.dataType();

if (type == DataType.DATETIME || type == DataType.DATE_NANOS) {
Rounding.Prepared preparedRounding = getDateRounding(toEvaluator.foldCtx());
return DateTrunc.evaluator(field.dataType(), source(), toEvaluator.apply(field), preparedRounding);
return DateTrunc.evaluator(type, source(), toEvaluator.apply(field), preparedRounding);
}
if (field.dataType().isNumeric()) {

if (type.isNumeric()) {
double roundTo;

if (from != null) {
int b = ((Number) buckets.fold(toEvaluator.foldCtx())).intValue();
double f = ((Number) from.fold(toEvaluator.foldCtx())).doubleValue();
double t = ((Number) to.fold(toEvaluator.foldCtx())).doubleValue();
roundTo = pickRounding(b, f, t);
int bucketCount = ((Number) buckets.fold(toEvaluator.foldCtx())).intValue();
double fromVal = ((Number) from.fold(toEvaluator.foldCtx())).doubleValue();
double toVal = ((Number) to.fold(toEvaluator.foldCtx())).doubleValue();
roundTo = pickRounding(bucketCount, fromVal, toVal);
} else {
roundTo = ((Number) buckets.fold(toEvaluator.foldCtx())).doubleValue();
}
Literal rounding = new Literal(source(), roundTo, DataType.DOUBLE);

// We could make this more efficient, either by generating the evaluators with byte code or hand rolling this one.
Literal rounding = new Literal(source(), roundTo, DataType.DOUBLE);
Div div = new Div(source(), field, rounding);
Floor floor = new Floor(source(), div);
Mul mul = new Mul(source(), floor, rounding);

return toEvaluator.apply(mul);
}
throw EsqlIllegalArgumentException.illegalDataType(field.dataType());

// Throw if the type is unsupported
throw EsqlIllegalArgumentException.illegalDataType(type);
}

/**
Expand Down
Loading