From 5e0cecc5e3c17f76d628a59737ea88e9a5483c58 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Mon, 20 Oct 2025 13:58:29 -0700 Subject: [PATCH 1/3] Fix bin nested fields issue Signed-off-by: Kai Huang --- .../sql/calcite/CalciteRelNodeVisitor.java | 11 +++- .../calcite/remote/CalciteBinCommandIT.java | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 06fc02c304d..7ecb9fcdf66 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -850,7 +850,16 @@ private void projectPlusOverriding( List originalFieldNames = context.relBuilder.peek().getRowType().getFieldNames(); List toOverrideList = originalFieldNames.stream() - .filter(newNames::contains) + .filter( + originalName -> + newNames.stream() + .anyMatch( + newName -> + // Match exact field names (e.g., "age" == "age") + // OR nested paths (e.g., "resource.attributes..." starts with + // "resource") + newName.equals(originalName) + || newName.startsWith(originalName + "."))) .map(a -> (RexNode) context.relBuilder.field(a)) .toList(); // 1. add the new fields, For example "age0, country0" diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java index 13e6b4a47e1..c021dae5316 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java @@ -27,6 +27,7 @@ public void init() throws Exception { loadIndex(Index.BANK); loadIndex(Index.EVENTS_NULL); loadIndex(Index.TIME_TEST_DATA); + loadIndex(Index.TELEMETRY); } @Test @@ -984,4 +985,56 @@ public void testStatsWithBinsOnTimeAndTermField_Avg() throws IOException { rows(50, "us-east", "2024-07-01 00:05:00"), rows(40.25, "us-west", "2024-07-01 00:01:00")); } + + @Test + public void testBinWithNestedFieldWithoutExplicitProjection() throws IOException { + // Test bin command on nested field without explicit fields projection + // This reproduces the bug from https://github.com/opensearch-project/sql/issues/4482 + // The telemetry index has: resource.attributes.telemetry.sdk.version (values: 10, 11, 12, 13, + // 14) + JSONObject result = + executeQuery( + String.format( + "source=%s | bin `resource.attributes.telemetry.sdk.version` span=2 | sort" + + " `resource.attributes.telemetry.sdk.version`", + TEST_INDEX_TELEMETRY)); + + // When binning a nested field, all sibling fields in the struct are also returned + verifySchema( + result, + schema("resource.attributes.telemetry.sdk.enabled", null, "boolean"), + schema("resource.attributes.telemetry.sdk.language", null, "string"), + schema("resource.attributes.telemetry.sdk.name", null, "string"), + schema("severityNumber", null, "int"), + schema("resource.attributes.telemetry.sdk.version", null, "string")); + + // With span=2 on values [10, 11, 12, 13, 14], we expect binned ranges: + // 10 -> 10-12, 11 -> 10-12, 12 -> 12-14, 13 -> 12-14, 14 -> 14-16 + // The binned field is the last column + verifyDataRows( + result, + rows(true, "java", "opentelemetry", 9, "10-12"), + rows(false, "python", "opentelemetry", 12, "10-12"), + rows(true, "javascript", "opentelemetry", 9, "12-14"), + rows(false, "go", "opentelemetry", 16, "12-14"), + rows(true, "rust", "opentelemetry", 12, "14-16")); + } + + @Test + public void testBinWithNestedFieldWithExplicitProjection() throws IOException { + // Test bin command on nested field WITH explicit fields projection (workaround) + // This is the workaround mentioned in https://github.com/opensearch-project/sql/issues/4482 + JSONObject result = + executeQuery( + String.format( + "source=%s | bin `resource.attributes.telemetry.sdk.version` span=2 | fields" + + " `resource.attributes.telemetry.sdk.version` | sort" + + " `resource.attributes.telemetry.sdk.version`", + TEST_INDEX_TELEMETRY)); + verifySchema(result, schema("resource.attributes.telemetry.sdk.version", null, "string")); + + // With span=2 on values [10, 11, 12, 13, 14], we expect binned ranges + verifyDataRows( + result, rows("10-12"), rows("10-12"), rows("12-14"), rows("12-14"), rows("14-16")); + } } From 58f20d85717bef125829a80de253c5422f7b5652 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Mon, 20 Oct 2025 19:00:47 -0700 Subject: [PATCH 2/3] refactor Signed-off-by: Kai Huang --- .../sql/calcite/CalciteRelNodeVisitor.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 7ecb9fcdf66..addb19ab602 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -850,16 +850,7 @@ private void projectPlusOverriding( List originalFieldNames = context.relBuilder.peek().getRowType().getFieldNames(); List toOverrideList = originalFieldNames.stream() - .filter( - originalName -> - newNames.stream() - .anyMatch( - newName -> - // Match exact field names (e.g., "age" == "age") - // OR nested paths (e.g., "resource.attributes..." starts with - // "resource") - newName.equals(originalName) - || newName.startsWith(originalName + "."))) + .filter(originalName -> shouldOverrideField(originalName, newNames)) .map(a -> (RexNode) context.relBuilder.field(a)) .toList(); // 1. add the new fields, For example "age0, country0" @@ -879,6 +870,17 @@ private void projectPlusOverriding( context.relBuilder.rename(expectedRenameFields); } + private boolean shouldOverrideField(String originalName, List newNames) { + return newNames.stream() + .anyMatch( + newName -> + // Match exact field names (e.g., "age" == "age") for flat fields + newName.equals(originalName) + // OR match nested paths (e.g., "resource.attributes..." starts with + // "resource.") + || newName.startsWith(originalName + ".")); + } + private List> extractInputRefList(List aggCalls) { return aggCalls.stream() .map(RelBuilder.AggCall::over) From c64bea553677f288d54dc19a1745852578e28a91 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Tue, 21 Oct 2025 19:38:06 -0700 Subject: [PATCH 3/3] add test Signed-off-by: Kai Huang --- .../calcite/remote/CalciteBinCommandIT.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java index c021dae5316..736f844aae9 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java @@ -1037,4 +1037,33 @@ public void testBinWithNestedFieldWithExplicitProjection() throws IOException { verifyDataRows( result, rows("10-12"), rows("10-12"), rows("12-14"), rows("12-14"), rows("14-16")); } + + @Test + public void testBinWithEvalCreatedDottedFieldName() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | eval `resource.temp` = 1 | bin" + + " `resource.attributes.telemetry.sdk.version` span=2 | sort" + + " `resource.attributes.telemetry.sdk.version`", + TEST_INDEX_TELEMETRY)); + + verifySchema( + result, + schema("resource.attributes.telemetry.sdk.enabled", null, "boolean"), + schema("resource.attributes.telemetry.sdk.language", null, "string"), + schema("resource.attributes.telemetry.sdk.name", null, "string"), + schema("resource.temp", null, "int"), + schema("severityNumber", null, "int"), + schema("resource.attributes.telemetry.sdk.version", null, "string")); + + // Data column order: enabled, language, name, severityNumber, resource.temp, version + verifyDataRows( + result, + rows(true, "java", "opentelemetry", 9, 1, "10-12"), + rows(false, "python", "opentelemetry", 12, 1, "10-12"), + rows(true, "javascript", "opentelemetry", 9, 1, "12-14"), + rows(false, "go", "opentelemetry", 16, 1, "12-14"), + rows(true, "rust", "opentelemetry", 12, 1, "14-16")); + } }