Skip to content

Commit 1a9a3a1

Browse files
authored
[Backport 2.19-dev] Fix sub-fields accessing of generated structs (#4683) (#4721)
* Fix sub-fields accessing of generated structs (#4683) * Correct subfield access logical when calling ITEM Signed-off-by: Yuanchun Shen <[email protected]> * Add explain and integration tests Signed-off-by: Yuanchun Shen <[email protected]> --------- Signed-off-by: Yuanchun Shen <[email protected]> (cherry picked from commit 3a42c51) * Mask host position in the plan of testInternalItemAccessOnStructs Signed-off-by: Yuanchun Shen <[email protected]> --------- Signed-off-by: Yuanchun Shen <[email protected]>
1 parent d20b4db commit 1a9a3a1

File tree

4 files changed

+49
-2
lines changed

4 files changed

+49
-2
lines changed

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,4 +1467,17 @@ public void testGeoIpPushedInAgg() throws IOException {
14671467
"source=%s | eval info = geoip('my-datasource', host) | stats count() by info.city",
14681468
TEST_INDEX_WEBLOGS)).replaceAll("\\$t?\\d+", "\\$*"));
14691469
}
1470+
1471+
@Test
1472+
public void testInternalItemAccessOnStructs() throws IOException {
1473+
String expected = loadExpectedPlan("access_struct_subfield_with_item.yaml");
1474+
assertYamlEqualsIgnoreId(
1475+
// The position of host in the scanned table is different in backport (no pushdown). Therefore, we mask all positions with $*
1476+
expected.replaceAll("\\$t?\\d+", "\\$*"),
1477+
explainQueryYaml(
1478+
String.format(
1479+
"source=%s | eval info = geoip('dummy-datasource', host) | fields host, info,"
1480+
+ " info.dummy_sub_field",
1481+
TEST_INDEX_WEBLOGS)).replaceAll("\\$t?\\d+", "\\$*"));
1482+
}
14701483
}

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteGeoIpFunctionsIT.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import org.opensearch.client.Request;
1919
import org.opensearch.sql.ppl.GeoIpFunctionsIT;
2020

21-
import java.io.IOException;
22-
2321
public class CalciteGeoIpFunctionsIT extends GeoIpFunctionsIT {
2422
@Override
2523
public void init() throws IOException {
@@ -86,4 +84,23 @@ public void testGeoIpInAggregation() throws IOException {
8684
verifySchema(result2, schema("count()", "bigint"), schema("info.city", "string"));
8785
verifyDataRows(result2, rows(1, "Seattle"), rows(1, "Bengaluru"));
8886
}
87+
88+
@Test
89+
public void testGeoIpEnrichmentAccessingSubField() throws IOException {
90+
JSONObject result =
91+
executeQuery(
92+
String.format(
93+
"source=%s | where method='POST' | eval info = geoip('%s', host) | fields host,"
94+
+ " info, info.country",
95+
TEST_INDEX_WEBLOGS, DATASOURCE_NAME));
96+
verifySchema(
97+
result, schema("host", "ip"), schema("info", "struct"), schema("info.country", "string"));
98+
verifyDataRows(
99+
result,
100+
rows("10.0.0.1", Map.of("country", "USA", "city", "Seattle"), "USA"),
101+
rows(
102+
"fd12:2345:6789:1:a1b2:c3d4:e5f6:789a",
103+
Map.of("country", "India", "city", "Bengaluru"),
104+
"India"));
105+
}
89106
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
calcite:
2+
logical: |
3+
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4+
LogicalProject(host=[$0], info=[GEOIP('dummy-datasource':VARCHAR, $0)], info.dummy_sub_field=[ITEM(GEOIP('dummy-datasource':VARCHAR, $0), 'dummy_sub_field')])
5+
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]])
6+
physical: |
7+
EnumerableCalc(expr#0=[{inputs}], expr#1=['dummy-datasource':VARCHAR], expr#2=[GEOIP($t1, $t0)], expr#3=['dummy_sub_field'], expr#4=[ITEM($t2, $t3)], host=[$t0], $f1=[$t2], $f2=[$t4])
8+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]], PushDownContext=[[PROJECT->[host], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","_source":{"includes":["host"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
calcite:
2+
logical: |
3+
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4+
LogicalProject(host=[$0], info=[GEOIP('dummy-datasource':VARCHAR, $0)], info.dummy_sub_field=[ITEM(GEOIP('dummy-datasource':VARCHAR, $0), 'dummy_sub_field')])
5+
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]])
6+
physical: |
7+
EnumerableLimit(fetch=[10000])
8+
EnumerableCalc(expr#0..11=[{inputs}], expr#12=['dummy-datasource':VARCHAR], expr#13=[GEOIP($t12, $t0)], expr#14=['dummy_sub_field'], expr#15=[ITEM($t13, $t14)], host=[$t0], info=[$t13], info.dummy_sub_field=[$t15])
9+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]])

0 commit comments

Comments
 (0)