From e30cceb062898a02a690e9138d7f828f25627c6d Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Wed, 5 Nov 2025 10:46:26 -0800 Subject: [PATCH 1/6] Update limitation and error message for bins parameter on time-related fields with pushdown disabled Signed-off-by: Kai Huang --- .../sql/calcite/utils/CalciteToolsHelper.java | 10 ++ .../binning/handlers/CountBinHandler.java | 2 +- .../sql/calcite/CalciteNoPushdownIT.java | 169 +++++++++--------- .../calcite/remote/CalciteBinCommandIT.java | 25 +++ .../opensearch/sql/ppl/PPLIntegTestCase.java | 4 + 5 files changed, 124 insertions(+), 86 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java index 6513f51a5a3..2a440eee726 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java @@ -355,6 +355,16 @@ public RelNode visit(TableScan scan) { final RelRunner runner = connection.unwrap(RelRunner.class); return runner.prepareStatement(rel); } catch (SQLException e) { + // Detect if error is due to window functions in unsupported context (bins on time fields) + String errorMsg = e.getMessage(); + if (errorMsg != null + && errorMsg.contains("Error while preparing plan") + && errorMsg.contains("WIDTH_BUCKET") + && (errorMsg.contains("OVER") || errorMsg.contains("window"))) { + throw new UnsupportedOperationException( + "The 'bins' parameter on timestamp fields is not supported when pushdown is" + + " disabled."); + } throw Util.throwAsRuntime(e); } } diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java index 7422a26f0b7..bd1f82aa5ff 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java @@ -28,7 +28,7 @@ public RexNode createExpression( CountBin countBin = (CountBin) node; // Create validated binnable field (validates that field is numeric or time-based) - // Note: bins parameter works with both numeric and time-based fields + // Note: bins parameter on time-based fields requires pushdown to be enabled String fieldName = BinFieldValidator.extractFieldName(node); BinnableField field = new BinnableField(fieldExpr, fieldExpr.getType(), fieldName); diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java index 15051417db1..83adfd07a72 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java @@ -10,7 +10,6 @@ import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.opensearch.sql.calcite.remote.*; -import org.opensearch.sql.calcite.tpch.CalcitePPLTpchIT; import org.opensearch.sql.ppl.PPLIntegTestCase; /** @@ -20,90 +19,90 @@ */ @RunWith(Suite.class) @Suite.SuiteClasses({ - CalciteExplainIT.class, - CalciteArrayFunctionIT.class, - CalciteBinCommandIT.class, - CalciteConvertTZFunctionIT.class, - CalciteCsvFormatIT.class, - CalciteDataTypeIT.class, - CalciteDateTimeComparisonIT.class, - CalciteDateTimeFunctionIT.class, - CalciteDateTimeImplementationIT.class, - CalciteDedupCommandIT.class, - CalciteDescribeCommandIT.class, - CalciteExpandCommandIT.class, - CalciteFieldsCommandIT.class, - CalciteFillNullCommandIT.class, - CalciteFlattenCommandIT.class, - CalciteFlattenDocValueIT.class, - CalciteGeoIpFunctionsIT.class, - CalciteGeoPointFormatsIT.class, - CalciteHeadCommandIT.class, - CalciteInformationSchemaCommandIT.class, - CalciteIPComparisonIT.class, - CalciteIPFunctionsIT.class, - CalciteJsonFunctionsIT.class, - CalciteLegacyAPICompatibilityIT.class, - CalciteLikeQueryIT.class, - CalciteMathematicalFunctionIT.class, - CalciteMultisearchCommandIT.class, - CalciteMultiValueStatsIT.class, - CalciteNewAddedCommandsIT.class, - CalciteNowLikeFunctionIT.class, - CalciteObjectFieldOperateIT.class, - CalciteOperatorIT.class, - CalciteParseCommandIT.class, - CalcitePPLAggregationIT.class, - CalcitePPLAppendcolIT.class, - CalcitePPLAppendCommandIT.class, - CalcitePPLBasicIT.class, - CalcitePPLBuiltinDatetimeFunctionInvalidIT.class, - CalcitePPLBuiltinFunctionIT.class, - CalcitePPLBuiltinFunctionsNullIT.class, - CalcitePPLCaseFunctionIT.class, - CalcitePPLCastFunctionIT.class, - CalcitePPLConditionBuiltinFunctionIT.class, - CalcitePPLCryptographicFunctionIT.class, - CalcitePPLDedupIT.class, - CalcitePPLEventstatsIT.class, - CalciteStreamstatsCommandIT.class, - CalcitePPLExistsSubqueryIT.class, - CalcitePPLExplainIT.class, - CalcitePPLFillnullIT.class, - CalcitePPLGrokIT.class, - CalcitePPLInSubqueryIT.class, - CalcitePPLIPFunctionIT.class, - CalcitePPLJoinIT.class, - CalcitePPLJsonBuiltinFunctionIT.class, - CalcitePPLLookupIT.class, - CalcitePPLParseIT.class, - CalcitePPLPatternsIT.class, - CalcitePPLPluginIT.class, - CalcitePPLRenameIT.class, - CalcitePPLScalarSubqueryIT.class, - CalcitePPLSortIT.class, - CalcitePPLStringBuiltinFunctionIT.class, - CalcitePPLTrendlineIT.class, - CalcitePrometheusDataSourceCommandsIT.class, - CalciteQueryAnalysisIT.class, - CalciteRareCommandIT.class, - CalciteRegexCommandIT.class, - CalciteRexCommandIT.class, - CalciteRenameCommandIT.class, - CalciteReplaceCommandIT.class, - CalciteResourceMonitorIT.class, - CalciteSearchCommandIT.class, - CalciteSettingsIT.class, - CalciteShowDataSourcesCommandIT.class, - CalciteSortCommandIT.class, - CalciteStatsCommandIT.class, - CalciteSystemFunctionIT.class, - CalciteTextFunctionIT.class, - CalciteTopCommandIT.class, - CalciteTrendlineCommandIT.class, - CalciteVisualizationFormatIT.class, - CalciteWhereCommandIT.class, - CalcitePPLTpchIT.class + // CalciteExplainIT.class, + // CalciteArrayFunctionIT.class, + CalciteBinCommandIT.class + // CalciteConvertTZFunctionIT.class, + // CalciteCsvFormatIT.class, + // CalciteDataTypeIT.class, + // CalciteDateTimeComparisonIT.class, + // CalciteDateTimeFunctionIT.class, + // CalciteDateTimeImplementationIT.class, + // CalciteDedupCommandIT.class, + // CalciteDescribeCommandIT.class, + // CalciteExpandCommandIT.class, + // CalciteFieldsCommandIT.class, + // CalciteFillNullCommandIT.class, + // CalciteFlattenCommandIT.class, + // CalciteFlattenDocValueIT.class, + // CalciteGeoIpFunctionsIT.class, + // CalciteGeoPointFormatsIT.class, + // CalciteHeadCommandIT.class, + // CalciteInformationSchemaCommandIT.class, + // CalciteIPComparisonIT.class, + // CalciteIPFunctionsIT.class, + // CalciteJsonFunctionsIT.class, + // CalciteLegacyAPICompatibilityIT.class, + // CalciteLikeQueryIT.class, + // CalciteMathematicalFunctionIT.class, + // CalciteMultisearchCommandIT.class, + // CalciteMultiValueStatsIT.class, + // CalciteNewAddedCommandsIT.class, + // CalciteNowLikeFunctionIT.class, + // CalciteObjectFieldOperateIT.class, + // CalciteOperatorIT.class, + // CalciteParseCommandIT.class, + // CalcitePPLAggregationIT.class, + // CalcitePPLAppendcolIT.class, + // CalcitePPLAppendCommandIT.class, + // CalcitePPLBasicIT.class, + // CalcitePPLBuiltinDatetimeFunctionInvalidIT.class, + // CalcitePPLBuiltinFunctionIT.class, + // CalcitePPLBuiltinFunctionsNullIT.class, + // CalcitePPLCaseFunctionIT.class, + // CalcitePPLCastFunctionIT.class, + // CalcitePPLConditionBuiltinFunctionIT.class, + // CalcitePPLCryptographicFunctionIT.class, + // CalcitePPLDedupIT.class, + // CalcitePPLEventstatsIT.class, + // CalciteStreamstatsCommandIT.class, + // CalcitePPLExistsSubqueryIT.class, + // CalcitePPLExplainIT.class, + // CalcitePPLFillnullIT.class, + // CalcitePPLGrokIT.class, + // CalcitePPLInSubqueryIT.class, + // CalcitePPLIPFunctionIT.class, + // CalcitePPLJoinIT.class, + // CalcitePPLJsonBuiltinFunctionIT.class, + // CalcitePPLLookupIT.class, + // CalcitePPLParseIT.class, + // CalcitePPLPatternsIT.class, + // CalcitePPLPluginIT.class, + // CalcitePPLRenameIT.class, + // CalcitePPLScalarSubqueryIT.class, + // CalcitePPLSortIT.class, + // CalcitePPLStringBuiltinFunctionIT.class, + // CalcitePPLTrendlineIT.class, + // CalcitePrometheusDataSourceCommandsIT.class, + // CalciteQueryAnalysisIT.class, + // CalciteRareCommandIT.class, + // CalciteRegexCommandIT.class, + // CalciteRexCommandIT.class, + // CalciteRenameCommandIT.class, + // CalciteReplaceCommandIT.class, + // CalciteResourceMonitorIT.class, + // CalciteSearchCommandIT.class, + // CalciteSettingsIT.class, + // CalciteShowDataSourcesCommandIT.class, + // CalciteSortCommandIT.class, + // CalciteStatsCommandIT.class, + // CalciteSystemFunctionIT.class, + // CalciteTextFunctionIT.class, + // CalciteTopCommandIT.class, + // CalciteTrendlineCommandIT.class, + // CalciteVisualizationFormatIT.class, + // CalciteWhereCommandIT.class, + // CalcitePPLTpchIT.class }) public class CalciteNoPushdownIT { private static boolean wasPushdownEnabled; 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 ef8abc3cba0..4545bb62d07 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 @@ -5,6 +5,7 @@ package org.opensearch.sql.calcite.remote; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.opensearch.sql.legacy.TestsConstants.*; import static org.opensearch.sql.util.MatcherUtils.rows; @@ -1126,6 +1127,30 @@ public void testStatsWithBinsOnTimeAndTermField_Avg() throws IOException { rows(40.25, "us-west", "2024-07-01 00:01:00")); } + @Test + public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOException { + // Verify that bins parameter on timestamp fields fails with clear error when pushdown disabled + // This is a known limitation: Calcite cannot execute window functions in expressions + enabledOnlyWhenPushdownIsDisabled(); + + ResponseException exception = + assertThrows( + ResponseException.class, + () -> + executeQuery( + "source=events_null | bin @timestamp bins=3 | stats count() by @timestamp")); + + // Verify the error message clearly explains the limitation and suggests solutions + String errorMessage = exception.getMessage(); + System.out.println("Debugging error message: " + errorMessage); + assertTrue( + "Expected clear error message about bins parameter not supported on timestamp fields when " + + "pushdown is disabled, but got: " + + errorMessage, + errorMessage.contains( + "bins' parameter on timestamp fields is not supported when pushdown is disabled")); + } + @Test public void testBinWithNestedFieldWithoutExplicitProjection() throws IOException { // Test bin command on nested field without explicit fields projection diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java index 5c2e45f1af1..0330d504a9d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java @@ -321,6 +321,10 @@ protected void enabledOnlyWhenPushdownIsEnabled() throws IOException { Assume.assumeTrue("This test is only for when push down is enabled", !isPushdownDisabled()); } + protected void enabledOnlyWhenPushdownIsDisabled() throws IOException { + Assume.assumeTrue("This test is only for when push down is disabled", isPushdownDisabled()); + } + public void updatePushdownSettings() throws IOException { String pushdownEnabled = String.valueOf(GlobalPushdownConfig.enabled); assert !pushdownEnabled.isBlank() : "Pushdown enabled setting cannot be empty"; From d2fb116e015d1143ca71c49ac3cc9069a1420b03 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Wed, 5 Nov 2025 13:47:02 -0800 Subject: [PATCH 2/6] Add documentation for bins parameter limitation on timestamp fields Signed-off-by: Kai Huang --- docs/user/ppl/cmd/bin.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/user/ppl/cmd/bin.rst b/docs/user/ppl/cmd/bin.rst index 1ebdc3f897e..a5bdf561a6b 100644 --- a/docs/user/ppl/cmd/bin.rst +++ b/docs/user/ppl/cmd/bin.rst @@ -182,6 +182,8 @@ Automatically calculates the span using a mathematical O(1) algorithm to create **Validation**: The bins parameter must be between 2 and 50000 (inclusive). Values outside this range will result in an error. +**Limitation**: The bins parameter on timestamp fields requires pushdown to be enabled. When pushdown is disabled, use the ``span`` parameter instead (e.g., ``bin @timestamp span=5m``). + The algorithm uses **mathematical optimization** instead of iteration for O(1) performance: 1. **Validate bins**: Ensure ``2 ≤ bins ≤ 50000`` From 92e4109550dc1fd47f836d9f0a62d6e1ac64a837 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Wed, 5 Nov 2025 13:59:12 -0800 Subject: [PATCH 3/6] fixes Signed-off-by: Kai Huang --- .../sql/calcite/utils/CalciteToolsHelper.java | 3 +- .../sql/calcite/CalciteNoPushdownIT.java | 169 +++++++++--------- .../calcite/remote/CalciteBinCommandIT.java | 2 - 3 files changed, 86 insertions(+), 88 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java index 2a440eee726..9d561a88cda 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java @@ -359,8 +359,7 @@ public RelNode visit(TableScan scan) { String errorMsg = e.getMessage(); if (errorMsg != null && errorMsg.contains("Error while preparing plan") - && errorMsg.contains("WIDTH_BUCKET") - && (errorMsg.contains("OVER") || errorMsg.contains("window"))) { + && errorMsg.contains("WIDTH_BUCKET")) { throw new UnsupportedOperationException( "The 'bins' parameter on timestamp fields is not supported when pushdown is" + " disabled."); diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java index 83adfd07a72..15051417db1 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java @@ -10,6 +10,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.opensearch.sql.calcite.remote.*; +import org.opensearch.sql.calcite.tpch.CalcitePPLTpchIT; import org.opensearch.sql.ppl.PPLIntegTestCase; /** @@ -19,90 +20,90 @@ */ @RunWith(Suite.class) @Suite.SuiteClasses({ - // CalciteExplainIT.class, - // CalciteArrayFunctionIT.class, - CalciteBinCommandIT.class - // CalciteConvertTZFunctionIT.class, - // CalciteCsvFormatIT.class, - // CalciteDataTypeIT.class, - // CalciteDateTimeComparisonIT.class, - // CalciteDateTimeFunctionIT.class, - // CalciteDateTimeImplementationIT.class, - // CalciteDedupCommandIT.class, - // CalciteDescribeCommandIT.class, - // CalciteExpandCommandIT.class, - // CalciteFieldsCommandIT.class, - // CalciteFillNullCommandIT.class, - // CalciteFlattenCommandIT.class, - // CalciteFlattenDocValueIT.class, - // CalciteGeoIpFunctionsIT.class, - // CalciteGeoPointFormatsIT.class, - // CalciteHeadCommandIT.class, - // CalciteInformationSchemaCommandIT.class, - // CalciteIPComparisonIT.class, - // CalciteIPFunctionsIT.class, - // CalciteJsonFunctionsIT.class, - // CalciteLegacyAPICompatibilityIT.class, - // CalciteLikeQueryIT.class, - // CalciteMathematicalFunctionIT.class, - // CalciteMultisearchCommandIT.class, - // CalciteMultiValueStatsIT.class, - // CalciteNewAddedCommandsIT.class, - // CalciteNowLikeFunctionIT.class, - // CalciteObjectFieldOperateIT.class, - // CalciteOperatorIT.class, - // CalciteParseCommandIT.class, - // CalcitePPLAggregationIT.class, - // CalcitePPLAppendcolIT.class, - // CalcitePPLAppendCommandIT.class, - // CalcitePPLBasicIT.class, - // CalcitePPLBuiltinDatetimeFunctionInvalidIT.class, - // CalcitePPLBuiltinFunctionIT.class, - // CalcitePPLBuiltinFunctionsNullIT.class, - // CalcitePPLCaseFunctionIT.class, - // CalcitePPLCastFunctionIT.class, - // CalcitePPLConditionBuiltinFunctionIT.class, - // CalcitePPLCryptographicFunctionIT.class, - // CalcitePPLDedupIT.class, - // CalcitePPLEventstatsIT.class, - // CalciteStreamstatsCommandIT.class, - // CalcitePPLExistsSubqueryIT.class, - // CalcitePPLExplainIT.class, - // CalcitePPLFillnullIT.class, - // CalcitePPLGrokIT.class, - // CalcitePPLInSubqueryIT.class, - // CalcitePPLIPFunctionIT.class, - // CalcitePPLJoinIT.class, - // CalcitePPLJsonBuiltinFunctionIT.class, - // CalcitePPLLookupIT.class, - // CalcitePPLParseIT.class, - // CalcitePPLPatternsIT.class, - // CalcitePPLPluginIT.class, - // CalcitePPLRenameIT.class, - // CalcitePPLScalarSubqueryIT.class, - // CalcitePPLSortIT.class, - // CalcitePPLStringBuiltinFunctionIT.class, - // CalcitePPLTrendlineIT.class, - // CalcitePrometheusDataSourceCommandsIT.class, - // CalciteQueryAnalysisIT.class, - // CalciteRareCommandIT.class, - // CalciteRegexCommandIT.class, - // CalciteRexCommandIT.class, - // CalciteRenameCommandIT.class, - // CalciteReplaceCommandIT.class, - // CalciteResourceMonitorIT.class, - // CalciteSearchCommandIT.class, - // CalciteSettingsIT.class, - // CalciteShowDataSourcesCommandIT.class, - // CalciteSortCommandIT.class, - // CalciteStatsCommandIT.class, - // CalciteSystemFunctionIT.class, - // CalciteTextFunctionIT.class, - // CalciteTopCommandIT.class, - // CalciteTrendlineCommandIT.class, - // CalciteVisualizationFormatIT.class, - // CalciteWhereCommandIT.class, - // CalcitePPLTpchIT.class + CalciteExplainIT.class, + CalciteArrayFunctionIT.class, + CalciteBinCommandIT.class, + CalciteConvertTZFunctionIT.class, + CalciteCsvFormatIT.class, + CalciteDataTypeIT.class, + CalciteDateTimeComparisonIT.class, + CalciteDateTimeFunctionIT.class, + CalciteDateTimeImplementationIT.class, + CalciteDedupCommandIT.class, + CalciteDescribeCommandIT.class, + CalciteExpandCommandIT.class, + CalciteFieldsCommandIT.class, + CalciteFillNullCommandIT.class, + CalciteFlattenCommandIT.class, + CalciteFlattenDocValueIT.class, + CalciteGeoIpFunctionsIT.class, + CalciteGeoPointFormatsIT.class, + CalciteHeadCommandIT.class, + CalciteInformationSchemaCommandIT.class, + CalciteIPComparisonIT.class, + CalciteIPFunctionsIT.class, + CalciteJsonFunctionsIT.class, + CalciteLegacyAPICompatibilityIT.class, + CalciteLikeQueryIT.class, + CalciteMathematicalFunctionIT.class, + CalciteMultisearchCommandIT.class, + CalciteMultiValueStatsIT.class, + CalciteNewAddedCommandsIT.class, + CalciteNowLikeFunctionIT.class, + CalciteObjectFieldOperateIT.class, + CalciteOperatorIT.class, + CalciteParseCommandIT.class, + CalcitePPLAggregationIT.class, + CalcitePPLAppendcolIT.class, + CalcitePPLAppendCommandIT.class, + CalcitePPLBasicIT.class, + CalcitePPLBuiltinDatetimeFunctionInvalidIT.class, + CalcitePPLBuiltinFunctionIT.class, + CalcitePPLBuiltinFunctionsNullIT.class, + CalcitePPLCaseFunctionIT.class, + CalcitePPLCastFunctionIT.class, + CalcitePPLConditionBuiltinFunctionIT.class, + CalcitePPLCryptographicFunctionIT.class, + CalcitePPLDedupIT.class, + CalcitePPLEventstatsIT.class, + CalciteStreamstatsCommandIT.class, + CalcitePPLExistsSubqueryIT.class, + CalcitePPLExplainIT.class, + CalcitePPLFillnullIT.class, + CalcitePPLGrokIT.class, + CalcitePPLInSubqueryIT.class, + CalcitePPLIPFunctionIT.class, + CalcitePPLJoinIT.class, + CalcitePPLJsonBuiltinFunctionIT.class, + CalcitePPLLookupIT.class, + CalcitePPLParseIT.class, + CalcitePPLPatternsIT.class, + CalcitePPLPluginIT.class, + CalcitePPLRenameIT.class, + CalcitePPLScalarSubqueryIT.class, + CalcitePPLSortIT.class, + CalcitePPLStringBuiltinFunctionIT.class, + CalcitePPLTrendlineIT.class, + CalcitePrometheusDataSourceCommandsIT.class, + CalciteQueryAnalysisIT.class, + CalciteRareCommandIT.class, + CalciteRegexCommandIT.class, + CalciteRexCommandIT.class, + CalciteRenameCommandIT.class, + CalciteReplaceCommandIT.class, + CalciteResourceMonitorIT.class, + CalciteSearchCommandIT.class, + CalciteSettingsIT.class, + CalciteShowDataSourcesCommandIT.class, + CalciteSortCommandIT.class, + CalciteStatsCommandIT.class, + CalciteSystemFunctionIT.class, + CalciteTextFunctionIT.class, + CalciteTopCommandIT.class, + CalciteTrendlineCommandIT.class, + CalciteVisualizationFormatIT.class, + CalciteWhereCommandIT.class, + CalcitePPLTpchIT.class }) public class CalciteNoPushdownIT { private static boolean wasPushdownEnabled; 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 4545bb62d07..e9feb81ef07 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 @@ -1130,7 +1130,6 @@ public void testStatsWithBinsOnTimeAndTermField_Avg() throws IOException { @Test public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOException { // Verify that bins parameter on timestamp fields fails with clear error when pushdown disabled - // This is a known limitation: Calcite cannot execute window functions in expressions enabledOnlyWhenPushdownIsDisabled(); ResponseException exception = @@ -1142,7 +1141,6 @@ public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOExcept // Verify the error message clearly explains the limitation and suggests solutions String errorMessage = exception.getMessage(); - System.out.println("Debugging error message: " + errorMessage); assertTrue( "Expected clear error message about bins parameter not supported on timestamp fields when " + "pushdown is disabled, but got: " From 2aa1768d699ec8093d9e52e5dcbfdc70e8262d47 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Thu, 6 Nov 2025 09:57:18 -0800 Subject: [PATCH 4/6] Update doc Signed-off-by: Kai Huang --- .../calcite/utils/binning/handlers/CountBinHandler.java | 7 ++++++- docs/user/ppl/cmd/bin.rst | 5 ++++- .../opensearch/sql/calcite/remote/CalciteBinCommandIT.java | 4 ++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java index bd1f82aa5ff..57b763b284d 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java @@ -28,7 +28,12 @@ public RexNode createExpression( CountBin countBin = (CountBin) node; // Create validated binnable field (validates that field is numeric or time-based) - // Note: bins parameter on time-based fields requires pushdown to be enabled + // Note: bins parameter works with both numeric and time-based fields + // Note: bins parameter on time-based fields requires: + // 1. Pushdown to be enabled (controlled by plugins.calcite.pushdown.enabled, enabled by + // default) + // 2. The time-based field to be used as an aggregation bucket + // (e.g., stats count() by @timestamp) String fieldName = BinFieldValidator.extractFieldName(node); BinnableField field = new BinnableField(fieldExpr, fieldExpr.getType(), fieldName); diff --git a/docs/user/ppl/cmd/bin.rst b/docs/user/ppl/cmd/bin.rst index a5bdf561a6b..2f7190dd41c 100644 --- a/docs/user/ppl/cmd/bin.rst +++ b/docs/user/ppl/cmd/bin.rst @@ -182,7 +182,10 @@ Automatically calculates the span using a mathematical O(1) algorithm to create **Validation**: The bins parameter must be between 2 and 50000 (inclusive). Values outside this range will result in an error. -**Limitation**: The bins parameter on timestamp fields requires pushdown to be enabled. When pushdown is disabled, use the ``span`` parameter instead (e.g., ``bin @timestamp span=5m``). +**Limitation**: The bins parameter on timestamp fields has the following requirements: + +1. **Pushdown must be enabled**: Controlled by ``plugins.calcite.pushdown.enabled`` (enabled by default). When pushdown is disabled, use the ``span`` parameter instead (e.g., ``bin @timestamp span=5m``). +2. **Timestamp field must be used as an aggregation bucket**: The binned timestamp field must be used in a ``stats`` aggregation (e.g., ``source=events | bin @timestamp bins=3 | stats count() by @timestamp``). Using bins on timestamp fields outside of aggregation buckets is not supported. The algorithm uses **mathematical optimization** instead of iteration for O(1) performance: 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 e9feb81ef07..db0a099a42d 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 @@ -1140,6 +1140,10 @@ public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOExcept "source=events_null | bin @timestamp bins=3 | stats count() by @timestamp")); // Verify the error message clearly explains the limitation and suggests solutions + // Note: bins parameter on timestamp fields requires BOTH: + // 1. Pushdown to be enabled (plugins.calcite.pushdown.enabled=true, enabled by default) + // 2. The timestamp field to be used as an aggregation bucket (e.g., stats count() by + // @timestamp) String errorMessage = exception.getMessage(); assertTrue( "Expected clear error message about bins parameter not supported on timestamp fields when " From bc510e793ea1e4b81c48c70ae2ed0ea2b0e620d0 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Thu, 6 Nov 2025 10:35:20 -0800 Subject: [PATCH 5/6] DCO Signed-off-by: Kai Huang --- .../sql/calcite/utils/binning/handlers/CountBinHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java index 57b763b284d..ea1ca6a15fc 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/binning/handlers/CountBinHandler.java @@ -28,7 +28,6 @@ public RexNode createExpression( CountBin countBin = (CountBin) node; // Create validated binnable field (validates that field is numeric or time-based) - // Note: bins parameter works with both numeric and time-based fields // Note: bins parameter on time-based fields requires: // 1. Pushdown to be enabled (controlled by plugins.calcite.pushdown.enabled, enabled by // default) From 6a17894341e0dd82079c5e573edf3fcc2a432553 Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Fri, 7 Nov 2025 14:14:45 -0800 Subject: [PATCH 6/6] update error message Signed-off-by: Kai Huang --- .../opensearch/sql/calcite/utils/CalciteToolsHelper.java | 6 ++++-- .../sql/calcite/remote/CalciteBinCommandIT.java | 9 +++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java index 9d561a88cda..b1b2efc966e 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java @@ -361,8 +361,10 @@ public RelNode visit(TableScan scan) { && errorMsg.contains("Error while preparing plan") && errorMsg.contains("WIDTH_BUCKET")) { throw new UnsupportedOperationException( - "The 'bins' parameter on timestamp fields is not supported when pushdown is" - + " disabled."); + "The 'bins' parameter on timestamp fields requires: (1) pushdown to be enabled" + + " (controlled by plugins.calcite.pushdown.enabled, enabled by default), and" + + " (2) the timestamp field to be used as an aggregation bucket (e.g., 'stats" + + " count() by @timestamp')."); } throw Util.throwAsRuntime(e); } 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 db0a099a42d..54888369f54 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 @@ -1146,11 +1146,12 @@ public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOExcept // @timestamp) String errorMessage = exception.getMessage(); assertTrue( - "Expected clear error message about bins parameter not supported on timestamp fields when " - + "pushdown is disabled, but got: " + "Expected clear error message about bins parameter requirements on timestamp fields, but" + + " got: " + errorMessage, - errorMessage.contains( - "bins' parameter on timestamp fields is not supported when pushdown is disabled")); + errorMessage.contains("bins' parameter on timestamp fields requires") + && errorMessage.contains("pushdown to be enabled") + && errorMessage.contains("aggregation bucket")); } @Test