Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions docs/changelog/127636.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
pr: 127636
summary: Disallow mixed quoted/unquoted patterns in FROM
area: ES|QL
type: breaking
issues:
- 122651
breaking:
title: Disallow mixed quoted/unquoted patterns in FROM
area: ES|QL
details: "Previously, the ES|QL grammar allowed users to individually quote constituent strings in index patterns\
\ such as \"remote_cluster\":\"index_name\". This would allow users to write complex malformed index patterns\
\ that often slip through grammar and the subsequent validation. This could result in runtime errors\
\ that can be misleading. This change simplifies the grammar to early reject such malformed index patterns\
\ at the parsing stage, allowing users to write simpler queries and see more relevant and meaningful\
\ errors."
impact: "Users can write queries with simpler index patterns and see more meaningful and relevant errors."
notable: false
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,12 @@ static CsvSpecReader.CsvTestCase convertToRemoteIndices(CsvSpecReader.CsvTestCas
String[] localIndices = fromStatement.substring("FROM ".length()).split(",");
final String remoteIndices;
if (canUseRemoteIndicesOnly() && randomBoolean()) {
remoteIndices = Arrays.stream(localIndices).map(index -> "*:" + index.trim()).collect(Collectors.joining(","));
remoteIndices = Arrays.stream(localIndices)
.map(index -> unquoteAndRequoteAsRemote(index.trim(), true))
.collect(Collectors.joining(","));
} else {
remoteIndices = Arrays.stream(localIndices)
.map(index -> "*:" + index.trim() + "," + index.trim())
.map(index -> unquoteAndRequoteAsRemote(index.trim(), false))
.collect(Collectors.joining(","));
}
var newFrom = "FROM " + remoteIndices + " " + commands[0].substring(fromStatement.length());
Expand All @@ -265,9 +267,13 @@ static CsvSpecReader.CsvTestCase convertToRemoteIndices(CsvSpecReader.CsvTestCas
assert parts.length >= 2 : commands[0];
String[] indices = parts[1].split(",");
if (canUseRemoteIndicesOnly() && randomBoolean()) {
parts[1] = Arrays.stream(indices).map(index -> "*:" + index.trim()).collect(Collectors.joining(","));
parts[1] = Arrays.stream(indices)
.map(index -> unquoteAndRequoteAsRemote(index.trim(), true))
.collect(Collectors.joining(","));
} else {
parts[1] = Arrays.stream(indices).map(index -> "*:" + index.trim() + "," + index.trim()).collect(Collectors.joining(","));
parts[1] = Arrays.stream(indices)
.map(index -> unquoteAndRequoteAsRemote(index.trim(), false))
.collect(Collectors.joining(","));
}
String newNewMetrics = String.join(" ", parts);
testCase.query = newNewMetrics + query.substring(first.length());
Expand Down Expand Up @@ -300,6 +306,40 @@ static boolean hasIndexMetadata(String query) {
return false;
}

/**
* Since partial quoting is prohibited, we need to take the index name, unquote it,
* convert it to a remote index, and then requote it. For example, "employees" is unquoted,
* turned into the remote index *:employees, and then requoted to get "*:employees".
* @param index Name of the index.
* @param asRemoteIndexOnly If the return needs to be in the form of "*:idx,idx" or "*:idx".
* @return A remote index pattern that's requoted.
*/
private static String unquoteAndRequoteAsRemote(String index, boolean asRemoteIndexOnly) {
index = index.trim();

int numOfQuotes = 0;
for (; numOfQuotes < index.length(); numOfQuotes++) {
if (index.charAt(numOfQuotes) != '"') {
break;
}
}

String unquoted = unquote(index, numOfQuotes);
if (asRemoteIndexOnly) {
return quote("*:" + unquoted, numOfQuotes);
} else {
return quote("*:" + unquoted + "," + unquoted, numOfQuotes);
}
}

private static String quote(String index, int numOfQuotes) {
return "\"".repeat(numOfQuotes) + index + "\"".repeat(numOfQuotes);
}

private static String unquote(String index, int numOfQuotes) {
return index.substring(numOfQuotes, index.length() - numOfQuotes);
}

@Override
protected boolean enableRoundingDoubleValuesOnAsserting() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,6 @@ public void testDataStreamPatterns() throws Exception {
testCases.put("test_ds_patterns*::data,test_ds_patterns*::failures,-test_ds_patterns_2*::data", 19L);
testCases.put("test_ds_patterns*::data,test_ds_patterns*::failures,-test_ds_patterns_2*::failures", 21L);

testCases.put("\"test_ds_patterns_1,test_ds_patterns_2\"::failures", 8L);

runDataStreamTest(testCases, new String[] { "test_ds_patterns_1", "test_ds_patterns_2", "test_ds_patterns_3" }, (key, value) -> {
try (var results = run("from " + key + " | stats count(@timestamp)")) {
assertEquals(key, 1, getValuesList(results).size());
Expand All @@ -1097,7 +1095,7 @@ public void testDataStreamInvalidPatterns() throws Exception {
// Only one selector separator is allowed per expression
testCases.put("::::data", "mismatched input '::' expecting {");
// Suffix case is not supported because there is no component named with the empty string
testCases.put("index::", "missing {QUOTED_STRING, UNQUOTED_SOURCE} at '|'");
testCases.put("index::", "missing UNQUOTED_SOURCE at '|'");

runDataStreamTest(testCases, new String[] { "test_ds_patterns_1" }, (key, value) -> {
logger.info(key);
Expand Down
11 changes: 7 additions & 4 deletions x-pack/plugin/esql/src/main/antlr/EsqlBaseParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -150,18 +150,21 @@ fromCommand
;

indexPattern
: (clusterString COLON)? indexString
| indexString (CAST_OP selectorString)?
: clusterString COLON unquotedIndexString
| unquotedIndexString CAST_OP selectorString
| indexString
;

clusterString
: UNQUOTED_SOURCE
| QUOTED_STRING
;

selectorString
: UNQUOTED_SOURCE
| QUOTED_STRING
;

unquotedIndexString
: UNQUOTED_SOURCE
;

indexString
Expand Down

Large diffs are not rendered by default.

Loading