Skip to content

Commit

Permalink
ESQL: Fix loading constant_keyword when unset (elastic#101496)
Browse files Browse the repository at this point in the history
When a `constant_keyword` field is first created you don't *have* to set
it's value - it'll default to `null` and then take on the value of the
first document that contains the field. Our block loading code didn't
like this state and would fail to run the query. This fixes that.

Closes elastic#101455
  • Loading branch information
nik9000 authored Oct 29, 2023
1 parent bd22bc9 commit fb03858
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,39 @@ constant_keyword:
- length: {values: 1}
- match: {values.0.0: 17}

---
constant_keyword with null value:
- do:
indices.create:
index: test
body:
mappings:
properties:
kind:
type: constant_keyword
color:
type: keyword

- do:
bulk:
index: test
refresh: true
body:
- { "index": { } }
- { "color": "red" }

- do:
esql.query:
body:
query: 'from test | limit 1'
- match: { columns.0.name: color }
- match: { columns.0.type: keyword }
- match: { columns.1.name: kind }
- match: { columns.1.type: keyword }
- length: { values: 1 }
- match: { values.0.0: red }
- match: { values.0.1: null }

---
multivalued keyword:
- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ public String familyTypeName() {
@Override
public BlockLoader blockLoader(BlockLoaderContext blContext) {
// TODO build a constant block directly
if (value == null) {
return BlockDocValuesReader.nulls();
}
BytesRef bytes = new BytesRef(value);
return context -> new BlockDocValuesReader() {
private int docId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@

package org.elasticsearch.xpack.constantkeyword.mapper;

import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.CheckedFunction;
import org.elasticsearch.index.mapper.BlockDocValuesReader;
import org.elasticsearch.index.mapper.BlockLoader;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentParsingException;
import org.elasticsearch.index.mapper.LuceneDocument;
Expand All @@ -22,7 +28,9 @@
import org.elasticsearch.index.mapper.MapperService.MergeReason;
import org.elasticsearch.index.mapper.MapperTestCase;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.TestBlock;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xpack.constantkeyword.ConstantKeywordMapperPlugin;
import org.elasticsearch.xpack.constantkeyword.mapper.ConstantKeywordFieldMapper.ConstantKeywordFieldType;
Expand All @@ -31,11 +39,13 @@
import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.Function;

import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.nullValue;

public class ConstantKeywordFieldMapperTests extends MapperTestCase {

Expand Down Expand Up @@ -214,6 +224,62 @@ protected boolean allowsNullValues() {
return false; // null is an error for constant keyword
}

/**
* Test loading blocks when there is no defined value. This is allowed
* for newly created indices that haven't received any documents that
* contain the field.
*/
public void testNullValueBlockLoaderReadValues() throws IOException {
testNullBlockLoader(blockReader -> (TestBlock) blockReader.readValues(TestBlock.FACTORY, TestBlock.docs(0)));
}

/**
* Test loading blocks when there is no defined value. This is allowed
* for newly created indices that haven't received any documents that
* contain the field.
*/
public void testNullValueBlockLoaderReadValuesFromSingleDoc() throws IOException {
testNullBlockLoader(blockReader -> {
TestBlock block = (TestBlock) blockReader.builder(TestBlock.FACTORY, 1);
blockReader.readValuesFromSingleDoc(0, block);
return block;
});
}

private void testNullBlockLoader(CheckedFunction<BlockDocValuesReader, TestBlock, IOException> body) throws IOException {
MapperService mapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("field");
b.field("type", "constant_keyword");
b.endObject();
}));
BlockLoader loader = mapper.fieldType("field").blockLoader(new MappedFieldType.BlockLoaderContext() {
@Override
public String indexName() {
throw new UnsupportedOperationException();
}

@Override
public SearchLookup lookup() {
throw new UnsupportedOperationException();
}

@Override
public Set<String> sourcePaths(String name) {
return mapper.mappingLookup().sourcePaths(name);
}
});
try (Directory directory = newDirectory()) {
RandomIndexWriter iw = new RandomIndexWriter(random(), directory);
LuceneDocument doc = mapper.documentMapper().parse(source(b -> {})).rootDoc();
iw.addDocument(doc);
iw.close();
try (DirectoryReader reader = DirectoryReader.open(directory)) {
TestBlock block = body.apply(loader.reader(reader.leaves().get(0)));
assertThat(block.get(0), nullValue());
}
}
}

@Override
protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) {
assertFalse("constant_keyword doesn't support ignore_malformed", ignoreMalformed);
Expand Down

0 comments on commit fb03858

Please sign in to comment.