Skip to content

Commit 9d329c4

Browse files
committed
Fix bin nested fields issue
Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent f6bb654 commit 9d329c4

File tree

2 files changed

+63
-1
lines changed

2 files changed

+63
-1
lines changed

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,16 @@ private void projectPlusOverriding(
850850
List<String> originalFieldNames = context.relBuilder.peek().getRowType().getFieldNames();
851851
List<RexNode> toOverrideList =
852852
originalFieldNames.stream()
853-
.filter(newNames::contains)
853+
.filter(
854+
originalName ->
855+
newNames.stream()
856+
.anyMatch(
857+
newName ->
858+
// Match exact field names (e.g., "age" == "age")
859+
// OR nested paths (e.g., "resource.attributes..." starts with
860+
// "resource")
861+
newName.equals(originalName)
862+
|| newName.startsWith(originalName + ".")))
854863
.map(a -> (RexNode) context.relBuilder.field(a))
855864
.toList();
856865
// 1. add the new fields, For example "age0, country0"

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public void init() throws Exception {
2727
loadIndex(Index.BANK);
2828
loadIndex(Index.EVENTS_NULL);
2929
loadIndex(Index.TIME_TEST_DATA);
30+
loadIndex(Index.TELEMETRY);
3031
}
3132

3233
@Test
@@ -984,4 +985,56 @@ public void testStatsWithBinsOnTimeAndTermField_Avg() throws IOException {
984985
rows(50, "us-east", "2024-07-01 00:05:00"),
985986
rows(40.25, "us-west", "2024-07-01 00:01:00"));
986987
}
988+
989+
@Test
990+
public void testBinWithNestedFieldWithoutExplicitProjection() throws IOException {
991+
// Test bin command on nested field without explicit fields projection
992+
// This reproduces the bug from https://github.com/opensearch-project/sql/issues/4482
993+
// The telemetry index has: resource.attributes.telemetry.sdk.version (values: 10, 11, 12, 13,
994+
// 14)
995+
JSONObject result =
996+
executeQuery(
997+
String.format(
998+
"source=%s | bin `resource.attributes.telemetry.sdk.version` span=2 | sort"
999+
+ " `resource.attributes.telemetry.sdk.version`",
1000+
TEST_INDEX_TELEMETRY));
1001+
1002+
// When binning a nested field, all sibling fields in the struct are also returned
1003+
verifySchema(
1004+
result,
1005+
schema("resource.attributes.telemetry.sdk.enabled", null, "boolean"),
1006+
schema("resource.attributes.telemetry.sdk.language", null, "string"),
1007+
schema("resource.attributes.telemetry.sdk.name", null, "string"),
1008+
schema("severityNumber", null, "int"),
1009+
schema("resource.attributes.telemetry.sdk.version", null, "string"));
1010+
1011+
// With span=2 on values [10, 11, 12, 13, 14], we expect binned ranges:
1012+
// 10 -> 10-12, 11 -> 10-12, 12 -> 12-14, 13 -> 12-14, 14 -> 14-16
1013+
// The binned field is the last column
1014+
verifyDataRows(
1015+
result,
1016+
rows(true, "java", "opentelemetry", 9, "10-12"),
1017+
rows(false, "python", "opentelemetry", 12, "10-12"),
1018+
rows(true, "javascript", "opentelemetry", 9, "12-14"),
1019+
rows(false, "go", "opentelemetry", 16, "12-14"),
1020+
rows(true, "rust", "opentelemetry", 12, "14-16"));
1021+
}
1022+
1023+
@Test
1024+
public void testBinWithNestedFieldWithExplicitProjection() throws IOException {
1025+
// Test bin command on nested field WITH explicit fields projection (workaround)
1026+
// This is the workaround mentioned in https://github.com/opensearch-project/sql/issues/4482
1027+
JSONObject result =
1028+
executeQuery(
1029+
String.format(
1030+
"source=%s | bin `resource.attributes.telemetry.sdk.version` span=2 | fields"
1031+
+ " `resource.attributes.telemetry.sdk.version` | sort"
1032+
+ " `resource.attributes.telemetry.sdk.version`",
1033+
TEST_INDEX_TELEMETRY));
1034+
verifySchema(result, schema("resource.attributes.telemetry.sdk.version", null, "string"));
1035+
1036+
// With span=2 on values [10, 11, 12, 13, 14], we expect binned ranges
1037+
verifyDataRows(
1038+
result, rows("10-12"), rows("10-12"), rows("12-14"), rows("12-14"), rows("14-16"));
1039+
}
9871040
}

0 commit comments

Comments
 (0)