Skip to content

SQL: add support for index aliases for SYS COLUMNS command #53525

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

Merged
merged 11 commits into from
Mar 17, 2020
Merged

SQL: add support for index aliases for SYS COLUMNS command #53525

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
947e501
Add support for index aliases for SYS COLUMNS command
astefan Mar 12, 2020
dabcccf
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Mar 12, 2020
3aa01f3
Update to test after branch update
astefan Mar 12, 2020
26bf119
Address reviews
astefan Mar 16, 2020
93d8162
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Mar 16, 2020
80f0b63
Fix one integration test after update branch
astefan Mar 16, 2020
d34271a
Address further review
astefan Mar 16, 2020
bf8df94
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Mar 16, 2020
9ed8fa2
Address more reviews
astefan Mar 17, 2020
cd6c420
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Mar 17, 2020
079be6b
Small updates
astefan Mar 17, 2020
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
236 changes: 215 additions & 21 deletions x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.xpack.ql.index;

import com.carrotsearch.hppc.cursors.ObjectCursor;
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;

import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.ActionListener;
Expand All @@ -16,12 +17,14 @@
import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.IndicesOptions.Option;
import org.elasticsearch.action.support.IndicesOptions.WildcardStates;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.xpack.ql.QlIllegalArgumentException;
Expand All @@ -42,9 +45,11 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
Expand Down Expand Up @@ -164,7 +169,6 @@ public boolean equals(Object obj) {
private final String clusterName;
private final DataTypeRegistry typeRegistry;


public IndexResolver(Client client, String clusterName, DataTypeRegistry typeRegistry) {
this.client = client;
this.clusterName = clusterName;
Expand Down Expand Up @@ -296,7 +300,7 @@ public static IndexResolution mergedMappings(DataTypeRegistry typeRegistry, Stri
}

// merge all indices onto the same one
List<EsIndex> indices = buildIndices(typeRegistry, indexNames, null, fieldCaps, i -> indexPattern, (n, types) -> {
List<EsIndex> indices = buildIndices(typeRegistry, indexNames, null, fieldCaps, null, i -> indexPattern, (n, types) -> {
StringBuilder errorMessage = new StringBuilder();

boolean hasUnmapped = types.containsKey(UNMAPPED);
Expand Down Expand Up @@ -473,17 +477,32 @@ private static FieldCapabilitiesRequest createFieldCapsRequest(String index, boo
public void resolveAsSeparateMappings(String indexWildcard, String javaRegex, boolean includeFrozen,
ActionListener<List<EsIndex>> listener) {
FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, includeFrozen);
client.fieldCaps(fieldRequest,
ActionListener.wrap(
response -> listener.onResponse(
separateMappings(typeRegistry, indexWildcard, javaRegex, response.getIndices(), response.get())),
listener::onFailure));
client.fieldCaps(fieldRequest, wrap(response -> {
client.admin().indices().getAliases(createGetAliasesRequest(response, includeFrozen), wrap(aliases ->
listener.onResponse(separateMappings(typeRegistry, javaRegex, response.getIndices(), response.get(), aliases.getAliases())),
ex -> {
if (ex instanceof IndexNotFoundException || ex instanceof ElasticsearchSecurityException) {
listener.onResponse(separateMappings(typeRegistry, javaRegex, response.getIndices(), response.get(), null));
} else {
listener.onFailure(ex);
}
}));
},
listener::onFailure));

}

public static List<EsIndex> separateMappings(DataTypeRegistry typeRegistry, String indexPattern, String javaRegex, String[] indexNames,
Map<String, Map<String, FieldCapabilities>> fieldCaps) {
return buildIndices(typeRegistry, indexNames, javaRegex, fieldCaps, Function.identity(), (s, cap) -> null);
private GetAliasesRequest createGetAliasesRequest(FieldCapabilitiesResponse response, boolean includeFrozen) {
return new GetAliasesRequest()
.local(true)
.aliases("*")
.indices(response.getIndices())
.indicesOptions(includeFrozen ? FIELD_CAPS_FROZEN_INDICES_OPTIONS : FIELD_CAPS_INDICES_OPTIONS);
}

public static List<EsIndex> separateMappings(DataTypeRegistry typeRegistry, String javaRegex, String[] indexNames,
Map<String, Map<String, FieldCapabilities>> fieldCaps, ImmutableOpenMap<String, List<AliasMetaData>> aliases) {
return buildIndices(typeRegistry, indexNames, javaRegex, fieldCaps, aliases, Function.identity(), (s, cap) -> null);
}

private static class Fields {
Expand All @@ -496,16 +515,27 @@ private static class Fields {
* each field.
*/
private static List<EsIndex> buildIndices(DataTypeRegistry typeRegistry, String[] indexNames, String javaRegex,
Map<String, Map<String, FieldCapabilities>> fieldCaps,
Map<String, Map<String, FieldCapabilities>> fieldCaps, ImmutableOpenMap<String, List<AliasMetaData>> aliases,
Function<String, String> indexNameProcessor,
BiFunction<String, Map<String, FieldCapabilities>, InvalidMappedField> validityVerifier) {

if (indexNames == null || indexNames.length == 0) {
if ((indexNames == null || indexNames.length == 0) && (aliases == null || aliases.size() == 0)) {
Copy link
Member

@costin costin Mar 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use org.elasticsearch.common.util.CollectionUtils.isEmpty instead.

Copy link
Contributor Author

@astefan astefan Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not possible unfortunately, because aliases is an ImmutableOpenMap, which is not a Collection.

return emptyList();
}

final List<String> resolvedIndices = asList(indexNames);
Map<String, Fields> indices = new LinkedHashMap<>(resolvedIndices.size());
Set<String> resolvedAliases = new HashSet<>();
if (aliases != null) {
Iterator<ObjectObjectCursor<String, List<AliasMetaData>>> iterator = aliases.iterator();
while (iterator.hasNext()) {
for(AliasMetaData alias : iterator.next().value) {
Copy link
Contributor

@matriv matriv Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for(AliasMetaData alias : iterator.next().value) {
for (AliasMetaData alias : iterator.next().value) {

resolvedAliases.add(alias.getAlias());
}
}
}

List<String> resolvedIndices = new ArrayList<>(asList(indexNames));
int mapSize = CollectionUtils.mapSize(resolvedIndices.size() + resolvedAliases.size());
Map<String, Fields> indices = new LinkedHashMap<>(mapSize);
Pattern pattern = javaRegex != null ? Pattern.compile(javaRegex) : null;

// sort fields in reverse order to build the field hierarchy
Expand All @@ -525,6 +555,8 @@ private static List<EsIndex> buildIndices(DataTypeRegistry typeRegistry, String[
Map<String, FieldCapabilities> types = new LinkedHashMap<>(entry.getValue());
// apply verification and possibly remove the "duplicate" CONSTANT_KEYWORD field type
final InvalidMappedField invalidField = validityVerifier.apply(fieldName, types);
// apply verification for fields belonging to index aliases
Map<String, InvalidMappedField> invalidFieldsForAliases = getInvalidFieldsForAliases(fieldName, types, aliases);

// filter meta fields and unmapped
FieldCapabilities unmapped = types.get(UNMAPPED);
Expand All @@ -545,7 +577,7 @@ private static List<EsIndex> buildIndices(DataTypeRegistry typeRegistry, String[
List<String> concreteIndices = null;
if (capIndices != null) {
if (unmappedIndices.isEmpty()) {
concreteIndices = asList(capIndices);
concreteIndices = new ArrayList<>(asList(capIndices));
} else {
concreteIndices = new ArrayList<>(capIndices.length);
for (String capIndex : capIndices) {
Expand All @@ -559,38 +591,63 @@ private static List<EsIndex> buildIndices(DataTypeRegistry typeRegistry, String[
concreteIndices = resolvedIndices;
}

// add to the list of concrete indices the aliases associated with these indices
Set<String> uniqueAliases = new LinkedHashSet<>();
if (aliases != null) {
for (String concreteIndex : concreteIndices) {
if (aliases.containsKey(concreteIndex)) {
List<AliasMetaData> concreteIndexAliases = aliases.get(concreteIndex);
concreteIndexAliases.stream().forEach(e -> uniqueAliases.add(e.alias()));
}
}
concreteIndices.addAll(uniqueAliases);
}

// put the field in their respective mappings
for (String index : concreteIndices) {
if (pattern == null || pattern.matcher(index).matches()) {
String indexName = indexNameProcessor.apply(index);
boolean isIndexAlias = uniqueAliases.contains(index);
if (pattern == null || pattern.matcher(index).matches() || isIndexAlias) {
String indexName = isIndexAlias ? index : indexNameProcessor.apply(index);
Fields indexFields = indices.get(indexName);
if (indexFields == null) {
indexFields = new Fields();
indices.put(indexName, indexFields);
}
EsField field = indexFields.flattedMapping.get(fieldName);
if (field == null || (invalidField != null && (field instanceof InvalidMappedField) == false)) {
boolean createField = false;
if (isIndexAlias == false) {
if (field == null || (invalidField != null && (field instanceof InvalidMappedField) == false)) {
createField = true;
}
}
else {
if (field == null && invalidFieldsForAliases.get(index) == null) {
createField = true;
}
}

if (createField) {
int dot = fieldName.lastIndexOf('.');
/*
* Looking up the "tree" at the parent fields here to see if the field is an alias.
* When the upper elements of the "tree" have no elements in fieldcaps, then this is an alias field. But not
* always: if there are two aliases - a.b.c.alias1 and a.b.c.alias2 - only one of them will be considered alias.
*/
Holder<Boolean> isAlias = new Holder<>(false);
Holder<Boolean> isAliasFieldType = new Holder<>(false);
if (dot >= 0) {
String parentName = fieldName.substring(0, dot);
if (indexFields.flattedMapping.get(parentName) == null) {
// lack of parent implies the field is an alias
if (fieldCaps.get(parentName) == null) {
isAlias.set(true);
isAliasFieldType.set(true);
}
}
}

createField(typeRegistry, fieldName, fieldCaps, indexFields.hierarchicalMapping, indexFields.flattedMapping,
s -> invalidField != null ? invalidField :
createField(typeRegistry, s, typeCap.getType(), emptyMap(), typeCap.isAggregatable(),
isAlias.get()));
isAliasFieldType.get()));
}
}
}
Expand All @@ -605,4 +662,141 @@ private static List<EsIndex> buildIndices(DataTypeRegistry typeRegistry, String[
foundIndices.sort(Comparator.comparing(EsIndex::name));
return foundIndices;
}


/*
* Checks if the field is valid (same type and same capabilities - searchable/aggregatable) across indices belonging to a list
* of aliases.
* A field can look like the example below (generated by field_caps API).
* "name": {
* "text": {
* "type": "text",
* "searchable": false,
* "aggregatable": false,
* "indices": [
* "bar",
* "foo"
* ],
* "non_searchable_indices": [
* "foo"
* ]
* },
* "keyword": {
* "type": "keyword",
* "searchable": false,
* "aggregatable": true,
* "non_aggregatable_indices": [
* "bar", "baz"
* ]
* }
* }
*/
private static Map<String, InvalidMappedField> getInvalidFieldsForAliases(String fieldName, Map<String, FieldCapabilities> types,
ImmutableOpenMap<String, List<AliasMetaData>> aliases) {
if (aliases == null || aliases.isEmpty()) {
return emptyMap();
}
Map<String, InvalidMappedField> invalidFields = new HashMap<>();
Map<String, Set<String>> typesErrors = new HashMap<>(); // map holding aliases and a list of unique field types accross its indices
Copy link
Contributor

@bpintea bpintea Mar 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/accross/across.

Map<String, Set<String>> aliasToIndices = new HashMap<>(); // map with aliases and their list of indices

Iterator<ObjectObjectCursor<String, List<AliasMetaData>>> iter = aliases.iterator();
while (iter.hasNext()) {
ObjectObjectCursor<String, List<AliasMetaData>> index = iter.next();
for (AliasMetaData aliasMetaData : index.value) {
String aliasName = aliasMetaData.alias();
aliasToIndices.putIfAbsent(aliasName, new HashSet<String>());
Copy link
Contributor

@matriv matriv Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be simply <>?

Suggested change
aliasToIndices.putIfAbsent(aliasName, new HashSet<String>());
aliasToIndices.putIfAbsent(aliasName, new HashSet<>());

aliasToIndices.get(aliasName).add(index.key);
}
}

// iterate over each type
for (Entry<String, FieldCapabilities> type : types.entrySet()) {
String esFieldType = type.getKey();
if (esFieldType == UNMAPPED) {
continue;
}
String[] indices = type.getValue().indices();
// if there is a list of indices where this field type is defined
if (indices != null) {
// Look at all these indices' aliases and add the type of the field to a list (Set) with unique elements.
// A valid mapping for a field in an index alias should contain only one type. If it doesn't, this means that field
// is mapped as different types across the indices in this index alias.
for (String index : indices) {
List<AliasMetaData> indexAliases = aliases.get(index);
if (indexAliases == null) {
continue;
}
for (AliasMetaData aliasMetaData : indexAliases) {
String aliasName = aliasMetaData.alias();
if (typesErrors.containsKey(aliasName)) {
typesErrors.get(aliasName).add(esFieldType);
} else {
Set<String> fieldTypes = new HashSet<>();
fieldTypes.add(esFieldType);
typesErrors.put(aliasName, fieldTypes);
}
}
}
}
}

for (String aliasName : aliasToIndices.keySet()) {
// if, for the same index alias, there are multiple field types for this fieldName ie the index alias has indices where the same
// field name is of different types
Set<String> esFieldTypes = typesErrors.get(aliasName);
if (esFieldTypes != null && esFieldTypes.size() > 1) {
// consider the field as invalid, for the currently checked index alias
// the error message doesn't actually matter
invalidFields.put(aliasName, new InvalidMappedField(fieldName));
} else {
// if the field type is the same across all this alias' indices, check the field's capabilities (searchable/aggregatable)
for (Entry<String, FieldCapabilities> type : types.entrySet()) {
if (type.getKey() == UNMAPPED) {
continue;
}
FieldCapabilities f = type.getValue();

// the existence of a list of non_aggregatable_indices is an indication that not all indices have the same capabilities
// but this list can contain indices belonging to other aliases, so we need to check only for this alias
if (f.nonAggregatableIndices() != null) {
Set<String> aliasIndices = aliasToIndices.get(aliasName);
int nonAggregatableCount = 0;
// either all or none of the non-aggregatable indices belonging to a certain alias should be in this list
for (String nonAggIndex : f.nonAggregatableIndices()) {
if (aliasIndices.contains(nonAggIndex)) {
nonAggregatableCount++;
}
}
if (nonAggregatableCount > 0 && nonAggregatableCount != aliasIndices.size()) {
invalidFields.put(aliasName, new InvalidMappedField(fieldName));
break;
}
}

// perform the same check for non_searchable_indices list
if (f.nonSearchableIndices() != null) {
Set<String> aliasIndices = aliasToIndices.get(aliasName);
int nonSearchableCount = 0;
// either all or none of the non-searchable indices belonging to a certain alias should be in this list
for (String nonSearchIndex : f.nonSearchableIndices()) {
if (aliasIndices.contains(nonSearchIndex)) {
nonSearchableCount++;
}
}
if (nonSearchableCount > 0 && nonSearchableCount != aliasIndices.size()) {
invalidFields.put(aliasName, new InvalidMappedField(fieldName));
break;
}
}
}
}
}

if (invalidFields.size() > 0) {
return invalidFields;
}
// everything checks
return emptyMap();
}
}
5 changes: 5 additions & 0 deletions x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/InvalidMappedField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ public InvalidMappedField(String name, String errorMessage) {
this.errorMessage = errorMessage;
}

public InvalidMappedField(String name) {
super(name, DataTypes.UNSUPPORTED, emptyMap(), false);
this.errorMessage = StringUtils.EMPTY;
}

public String errorMessage() {
return errorMessage;
}
Expand Down
13 changes: 13 additions & 0 deletions ...qa/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/SysColumnsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.sql.qa.single_node;

import org.elasticsearch.xpack.sql.qa.jdbc.SysColumnsTestCase;

public class SysColumnsIT extends SysColumnsTestCase {

}
Loading