Skip to content

QL: handle IP type fields extraction with ignore_malformed property (#66622) #66686

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 1 commit into from
Dec 21, 2020
Merged

QL: handle IP type fields extraction with ignore_malformed property (#66622) #66686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 5 additions & 8 deletions ...java/org/elasticsearch/xpack/ql/execution/search/extractor/AbstractFieldHitExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,13 @@ public Object extract(SearchHit hit) {
// if the field was ignored because it was malformed and ignore_malformed was turned on
if (fullFieldName != null
&& hit.getFields().containsKey(IgnoredFieldMapper.NAME)
&& isFromDocValuesOnly(dataType) == false
&& dataType.isNumeric()) {
&& isFromDocValuesOnly(dataType) == false) {
/*
* ignore_malformed makes sense for extraction from _source for numeric fields only.
* And we check here that the data type is actually a numeric one to rule out
* any non-numeric sub-fields (for which the "parent" field should actually be extracted from _source).
* We check here the presence of the field name (fullFieldName including the parent name) in the list
* of _ignored fields (due to malformed data, which was ignored).
* For example, in the case of a malformed number, a "byte" field with "ignore_malformed: true"
* with a "text" sub-field should return "null" for the "byte" parent field and the actual malformed
* data for the "text" sub-field. Also, the _ignored section of the response contains the full field
* name, thus the need to do the comparison with that and not only the field name.
* data for the "text" sub-field.
*/
if (hit.getFields().get(IgnoredFieldMapper.NAME).getValues().contains(fullFieldName)) {
return null;
Expand Down Expand Up @@ -202,7 +199,7 @@ protected Object unwrapMultiValue(Object values) {
}
return result;
}
} else if (DataTypes.isString(dataType)) {
} else if (DataTypes.isString(dataType) || dataType == DataTypes.IP) {
return values.toString();
} else {
return values;
Expand Down
278 changes: 274 additions & 4 deletions ...in/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,25 +345,155 @@ public void testBooleanField() throws IOException {

/*
* "ip_field": {
* "type": "ip"
* "type": "ip",
* "ignore_malformed": true/false
* }
*/
public void testIpField() throws IOException {
String query = "SELECT ip_field FROM test";
String ipField = "192.168.1.1";
String actualValue = ipField;
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean();

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

createIndexWithFieldTypeAndProperties("ip", null, explicitSourceSetting ? indexProps : null);
index("{\"ip_field\":\"" + ipField + "\"}");
Map<String, Map<String, Object>> fieldProps = null;
if (ignoreMalformed) {
fieldProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-IP and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
fieldProps.put("ip_field", fieldProp);
actualValue = "foo";
}
createIndexWithFieldTypeAndProperties("ip", fieldProps, explicitSourceSetting ? indexProps : null);
index("{\"ip_field\":\"" + actualValue + "\"}");

if (explicitSourceSetting == false || enableSource) {
Map<String, Object> expected = new HashMap<>();
expected.put("columns", Arrays.asList(columnInfo("plain", "ip_field", "ip", JDBCType.VARCHAR, Integer.MAX_VALUE)));
expected.put("rows", singletonList(singletonList(ipField)));
expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : actualValue)));
assertResponse(expected, runSql(query));
} else {
expectSourceDisabledError(query);
}
}

/*
* "geo_point_field": {
* "type": "geo_point",
* "ignore_malformed": true/false
* }
*/
public void testGeoPointField() throws IOException {
String query = "SELECT geo_point_field FROM test";
String geoPointField = "41.12,-71.34";
String geoPointFromDocValues = "POINT (-71.34000004269183 41.1199999647215)";
String actualValue = geoPointField;
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean();

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

Map<String, Map<String, Object>> fieldProps = null;
if (ignoreMalformed) {
fieldProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-geo-point and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
fieldProps.put("geo_point_field", fieldProp);
actualValue = "foo";
}
createIndexWithFieldTypeAndProperties("geo_point", fieldProps, explicitSourceSetting ? indexProps : null);
index("{\"geo_point_field\":\"" + actualValue + "\"}");

// the values come from docvalues (vs from _source) so disabling the source doesn't have any impact on the values returned
Map<String, Object> expected = new HashMap<>();
expected.put("columns", Arrays.asList(columnInfo("plain", "geo_point_field", "geo_point", JDBCType.VARCHAR, Integer.MAX_VALUE)));
expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : geoPointFromDocValues)));
assertResponse(expected, runSql(query));
}

/*
* "geo_shape_field": {
* "type": "point",
* "ignore_malformed": true/false
* }
*/
public void testGeoShapeField() throws IOException {
String query = "SELECT geo_shape_field FROM test";
String actualValue = "[-77.03653, 38.897676]";
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean();

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

Map<String, Map<String, Object>> fieldProps = null;
if (ignoreMalformed) {
fieldProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-geo-shape and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
fieldProps.put("geo_shape_field", fieldProp);
actualValue = "\"foo\"";
}
createIndexWithFieldTypeAndProperties("geo_shape", fieldProps, explicitSourceSetting ? indexProps : null);
index("{\"geo_shape_field\":{\"type\":\"point\",\"coordinates\":" + actualValue + "}}");

if (explicitSourceSetting == false || enableSource) {
Map<String, Object> expected = new HashMap<>();
expected.put(
"columns",
Arrays.asList(columnInfo("plain", "geo_shape_field", "geo_shape", JDBCType.VARCHAR, Integer.MAX_VALUE))
);
expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : "POINT (-77.03653 38.897676)")));
assertResponse(expected, runSql(query));
} else {
expectSourceDisabledError(query);
}
}

/*
* "shape_field": {
* "type": "shape",
* "ignore_malformed": true/false
* }
*/
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/66678")
public void testShapeField() throws IOException {
String query = "SELECT shape_field FROM test";
String shapeField = "POINT (-377.03653 389.897676)";
String actualValue = shapeField;
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean();

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

Map<String, Map<String, Object>> fieldProps = null;
if (ignoreMalformed) {
fieldProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-geo-point and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
fieldProps.put("shape_field", fieldProp);
actualValue = "foo";
}
createIndexWithFieldTypeAndProperties("shape", fieldProps, explicitSourceSetting ? indexProps : null);
index("{\"shape_field\":\"" + actualValue + "\"}");

if (explicitSourceSetting == false || enableSource) {
Map<String, Object> expected = new HashMap<>();
expected.put("columns", Arrays.asList(columnInfo("plain", "shape_field", "shape", JDBCType.VARCHAR, Integer.MAX_VALUE)));
expected.put("rows", singletonList(singletonList(ignoreMalformed ? null : "POINT (-377.03653 389.897676)")));
assertResponse(expected, runSql(query));
} else {
expectSourceDisabledError(query);
Expand Down Expand Up @@ -584,6 +714,65 @@ public void testTextFieldWithIntegerNumberSubfield() throws IOException {
}
}

/*
* "text_field": {
* "type": "text",
* "fields": {
* "ip_subfield": {
* "type": "ip",
* "ignore_malformed": true/false
* }
* }
* }
*/
public void testTextFieldWithIpSubfield() throws IOException {
String ip = "123.123.123.123";
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean(); // ignore_malformed is true, thus test a non-IP value
String actualValue = ip;
String fieldName = "text_field";
String subFieldName = "text_field.ip_subfield";
String query = "SELECT " + fieldName + "," + subFieldName + " FROM test";

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

Map<String, Map<String, Object>> subFieldsProps = null;
if (ignoreMalformed) {
subFieldsProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-IP value instead of an IP and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
subFieldsProps.put(subFieldName, fieldProp);
actualValue = "foo";
}

createIndexWithFieldTypeAndSubFields("text", null, explicitSourceSetting ? indexProps : null, subFieldsProps, "ip");
index("{\"" + fieldName + "\":\"" + actualValue + "\"}");

if (explicitSourceSetting == false || enableSource) {
Map<String, Object> expected = new HashMap<>();
expected.put(
"columns",
Arrays.asList(
columnInfo("plain", fieldName, "text", JDBCType.VARCHAR, Integer.MAX_VALUE),
columnInfo("plain", subFieldName, "ip", JDBCType.VARCHAR, Integer.MAX_VALUE)
)
);
if (ignoreMalformed) {
expected.put("rows", singletonList(Arrays.asList("foo", null)));
} else {
expected.put("rows", singletonList(Arrays.asList(ip, ip)));
}
assertResponse(expected, runSql(query));
} else {
expectSourceDisabledError(query);
// if the _source is disabled, selecting only the ip sub-field shouldn't work as well
expectSourceDisabledError("SELECT " + subFieldName + " FROM test");
}
}

/*
* "integer_field": {
* "type": "integer",
Expand Down Expand Up @@ -663,6 +852,85 @@ public void testNumberFieldWithTextOrKeywordSubfield() throws IOException {
}
}

/*
* "ip_field": {
* "type": "ip",
* "ignore_malformed": true/false,
* "fields": {
* "keyword_subfield/text_subfield": {
* "type": "keyword/text"
* }
* }
* }
*/
public void testIpFieldWithTextOrKeywordSubfield() throws IOException {
String ip = "123.123.123.123";
boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting
boolean enableSource = randomBoolean(); // enable _source at index level
boolean ignoreMalformed = randomBoolean(); // ignore_malformed is true, thus test a non-number value
boolean isKeyword = randomBoolean(); // text or keyword subfield
String actualValue = ip;
String fieldName = "ip_field";
String subFieldName = "ip_field." + (isKeyword ? "keyword_subfield" : "text_subfield");
String query = "SELECT " + fieldName + "," + subFieldName + " FROM test";

Map<String, Object> indexProps = new HashMap<>(1);
indexProps.put("_source", enableSource);

Map<String, Map<String, Object>> fieldProps = null;
if (ignoreMalformed) {
fieldProps = new HashMap<>(1);
Map<String, Object> fieldProp = new HashMap<>(1);
// on purpose use a non-IP instead of an ip and check for null when querying the field's value
fieldProp.put("ignore_malformed", true);
fieldProps.put(fieldName, fieldProp);
actualValue = "foo";
}

createIndexWithFieldTypeAndSubFields(
"ip",
fieldProps,
explicitSourceSetting ? indexProps : null,
null,
isKeyword ? "keyword" : "text"
);
index("{\"" + fieldName + "\":\"" + actualValue + "\"}");

if (explicitSourceSetting == false || enableSource) {
Map<String, Object> expected = new HashMap<>();
expected.put(
"columns",
Arrays.asList(
columnInfo("plain", fieldName, "ip", JDBCType.VARCHAR, Integer.MAX_VALUE),
columnInfo("plain", subFieldName, isKeyword ? "keyword" : "text", JDBCType.VARCHAR, Integer.MAX_VALUE)
)
);
if (ignoreMalformed) {
expected.put("rows", singletonList(Arrays.asList(null, "foo")));
} else {
expected.put("rows", singletonList(Arrays.asList(ip, ip)));
}
assertResponse(expected, runSql(query));
} else {
if (isKeyword) {
// selecting only the keyword subfield when the _source is disabled should work
Map<String, Object> expected = new HashMap<>();
expected.put("columns", singletonList(columnInfo("plain", subFieldName, "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE)));
if (ignoreMalformed) {
expected.put("rows", singletonList(singletonList("foo")));
} else {
expected.put("rows", singletonList(singletonList(ip)));
}
assertResponse(expected, runSql("SELECT ip_field.keyword_subfield FROM test"));
} else {
expectSourceDisabledError(query);
}

// if the _source is disabled, selecting only the ip field shouldn't work
expectSourceDisabledError("SELECT " + fieldName + " FROM test");
}
}

/*
* "integer_field": {
* "type": "integer",
Expand Down Expand Up @@ -946,6 +1214,8 @@ private JDBCType jdbcTypeFor(String esType) {
return JDBCType.FLOAT;
case "scaled_float":
return JDBCType.DOUBLE;
case "ip":
return JDBCType.VARCHAR;
default:
throw new AssertionError("Illegal value [" + esType + "] for data type");
}
Expand Down