Skip to content

Commit b136683

Browse files
authored
Merge branch 'main' into issues/4631
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
2 parents 40972d6 + 20f2234 commit b136683

73 files changed

Lines changed: 4926 additions & 439 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/enforce-labels.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ jobs:
99
steps:
1010
- uses: yogevbd/enforce-label-action@2.1.0
1111
with:
12-
REQUIRED_LABELS_ANY: "breaking,feature,enhancement,bug,infrastructure,dependencies,documentation,maintenance,skip-changelog,testing,security fix"
13-
REQUIRED_LABELS_ANY_DESCRIPTION: "A release label is required: ['breaking', 'bug', 'dependencies', 'documentation', 'enhancement', 'feature', 'infrastructure', 'maintenance', 'skip-changelog', 'testing', 'security fix']"
12+
REQUIRED_LABELS_ANY: "breaking,feature,enhancement,bugFix,infrastructure,dependencies,documentation,maintenance,skip-changelog,testing,security fix"
13+
REQUIRED_LABELS_ANY_DESCRIPTION: "A release label is required: ['breaking', 'bugFix', 'dependencies', 'documentation', 'enhancement', 'feature', 'infrastructure', 'maintenance', 'skip-changelog', 'testing', 'security fix']"

.github/workflows/integ-tests-with-security.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ jobs:
6262
strategy:
6363
fail-fast: false
6464
matrix:
65-
os: [ windows-latest, macos-13 ]
65+
os: [ windows-latest, macos-14 ]
6666
java: [21, 24]
6767

6868
runs-on: ${{ matrix.os }}

.github/workflows/sql-test-and-build-workflow.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ jobs:
107107
entry:
108108
- { os: windows-latest, java: 21, os_build_args: -PbuildPlatform=windows }
109109
- { os: windows-latest, java: 24, os_build_args: -PbuildPlatform=windows }
110-
- { os: macos-13, java: 21, os_build_args: '' }
111-
- { os: macos-13, java: 24, os_build_args: '' }
110+
- { os: macos-14, java: 21, os_build_args: '' }
111+
- { os: macos-14, java: 24, os_build_args: '' }
112112
test-type: ['unit', 'integration', 'doc']
113113
exclude:
114114
# Exclude doctest for Windows
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.calcite.plan;
7+
8+
import org.apache.calcite.plan.Contexts;
9+
import org.apache.calcite.plan.RelRule;
10+
import org.apache.calcite.rel.core.RelFactories;
11+
import org.apache.calcite.tools.RelBuilderFactory;
12+
import org.immutables.value.Value;
13+
import org.opensearch.sql.calcite.utils.CalciteToolsHelper;
14+
15+
public interface OpenSearchRuleConfig extends RelRule.Config {
16+
17+
/** Return a custom RelBuilderFactory for creating OpenSearchRelBuilder */
18+
@Override
19+
@Value.Default
20+
default RelBuilderFactory relBuilderFactory() {
21+
return CalciteToolsHelper.proto(Contexts.of(RelFactories.DEFAULT_STRUCT));
22+
}
23+
}

core/src/main/java/org/opensearch/sql/calcite/plan/PPLAggGroupMergeRule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void apply(RelOptRuleCall call, LogicalAggregate aggregate, LogicalProjec
105105

106106
/** Rule configuration. */
107107
@Value.Immutable
108-
public interface Config extends RelRule.Config {
108+
public interface Config extends OpenSearchRuleConfig {
109109
Config GROUP_MERGE =
110110
ImmutablePPLAggGroupMergeRule.Config.builder()
111111
.build()

core/src/main/java/org/opensearch/sql/calcite/plan/PPLAggregateConvertRule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ private RexNode aliasMaybe(RelBuilder builder, RexNode node, String alias) {
240240

241241
/** Rule configuration. */
242242
@Value.Immutable
243-
public interface Config extends RelRule.Config {
243+
public interface Config extends OpenSearchRuleConfig {
244244
Config SUM_CONVERTER =
245245
ImmutablePPLAggregateConvertRule.Config.builder()
246246
.build()

core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
import org.apache.calcite.tools.FrameworkConfig;
8484
import org.apache.calcite.tools.Frameworks;
8585
import org.apache.calcite.tools.RelBuilder;
86+
import org.apache.calcite.tools.RelBuilderFactory;
8687
import org.apache.calcite.tools.RelRunner;
8788
import org.apache.calcite.util.Holder;
8889
import org.apache.calcite.util.Util;
@@ -127,6 +128,10 @@ public static Connection connect(FrameworkConfig config, JavaTypeFactory typeFac
127128
}
128129
}
129130

131+
public static RelBuilderFactory proto(final Context context) {
132+
return (cluster, schema) -> new OpenSearchRelBuilder(context, cluster, schema);
133+
}
134+
130135
/**
131136
* This method copied from {@link Frameworks#withPrepare(FrameworkConfig,
132137
* Frameworks.BasePrepareAction)}. The purpose is the method {@link

core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,12 +338,12 @@ public static boolean isUserDefinedType(RelDataType type) {
338338
}
339339

340340
/**
341-
* Checks if the RelDataType represents a numeric field. Supports both standard SQL numeric types
342-
* (INTEGER, BIGINT, SMALLINT, TINYINT, FLOAT, DOUBLE, DECIMAL, REAL) and OpenSearch UDT numeric
343-
* types.
341+
* Checks if the RelDataType represents a numeric type. Supports standard SQL numeric types
342+
* (INTEGER, BIGINT, SMALLINT, TINYINT, FLOAT, DOUBLE, DECIMAL, REAL), OpenSearch UDT numeric
343+
* types, and string types (VARCHAR, CHAR).
344344
*
345345
* @param fieldType the RelDataType to check
346-
* @return true if the type is numeric, false otherwise
346+
* @return true if the type is numeric or string, false otherwise
347347
*/
348348
public static boolean isNumericType(RelDataType fieldType) {
349349
// Check standard SQL numeric types
@@ -359,6 +359,11 @@ public static boolean isNumericType(RelDataType fieldType) {
359359
return true;
360360
}
361361

362+
// Check string types (VARCHAR, CHAR)
363+
if (sqlType == SqlTypeName.VARCHAR || sqlType == SqlTypeName.CHAR) {
364+
return true;
365+
}
366+
362367
// Check for OpenSearch UDT numeric types
363368
if (isUserDefinedType(fieldType)) {
364369
AbstractExprRelDataType<?> exprType = (AbstractExprRelDataType<?>) fieldType;

core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,65 @@ private PPLOperandTypes() {}
134134
SqlTypeFamily.NUMERIC,
135135
SqlTypeFamily.NUMERIC,
136136
SqlTypeFamily.NUMERIC));
137+
138+
public static final UDFOperandMetadata WIDTH_BUCKET_OPERAND =
139+
UDFOperandMetadata.wrap(
140+
(CompositeOperandTypeChecker)
141+
// 1. Numeric fields: bin age span=10
142+
OperandTypes.family(
143+
SqlTypeFamily.NUMERIC,
144+
SqlTypeFamily.INTEGER,
145+
SqlTypeFamily.NUMERIC,
146+
SqlTypeFamily.NUMERIC)
147+
// 2. Timestamp fields with OpenSearch type system
148+
// Used in: Production + Integration tests (CalciteBinCommandIT)
149+
.or(
150+
OperandTypes.family(
151+
SqlTypeFamily.TIMESTAMP,
152+
SqlTypeFamily.INTEGER,
153+
SqlTypeFamily.CHARACTER, // TIMESTAMP - TIMESTAMP = INTERVAL (as STRING)
154+
SqlTypeFamily.TIMESTAMP))
155+
// 3. Timestamp fields with Calcite SCOTT schema
156+
// Used in: Unit tests (CalcitePPLBinTest)
157+
.or(
158+
OperandTypes.family(
159+
SqlTypeFamily.TIMESTAMP,
160+
SqlTypeFamily.INTEGER,
161+
SqlTypeFamily.TIMESTAMP, // TIMESTAMP - TIMESTAMP = TIMESTAMP
162+
SqlTypeFamily.TIMESTAMP))
163+
// DATE field with OpenSearch type system
164+
// Used in: Production + Integration tests (CalciteBinCommandIT)
165+
.or(
166+
OperandTypes.family(
167+
SqlTypeFamily.DATE,
168+
SqlTypeFamily.INTEGER,
169+
SqlTypeFamily.CHARACTER, // DATE - DATE = INTERVAL (as STRING)
170+
SqlTypeFamily.DATE))
171+
// DATE field with Calcite SCOTT schema
172+
// Used in: Unit tests (CalcitePPLBinTest)
173+
.or(
174+
OperandTypes.family(
175+
SqlTypeFamily.DATE,
176+
SqlTypeFamily.INTEGER,
177+
SqlTypeFamily.DATE, // DATE - DATE = DATE
178+
SqlTypeFamily.DATE))
179+
// TIME field with OpenSearch type system
180+
// Used in: Production + Integration tests (CalciteBinCommandIT)
181+
.or(
182+
OperandTypes.family(
183+
SqlTypeFamily.TIME,
184+
SqlTypeFamily.INTEGER,
185+
SqlTypeFamily.CHARACTER, // TIME - TIME = INTERVAL (as STRING)
186+
SqlTypeFamily.TIME))
187+
// TIME field with Calcite SCOTT schema
188+
// Used in: Unit tests (CalcitePPLBinTest)
189+
.or(
190+
OperandTypes.family(
191+
SqlTypeFamily.TIME,
192+
SqlTypeFamily.INTEGER,
193+
SqlTypeFamily.TIME, // TIME - TIME = TIME
194+
SqlTypeFamily.TIME)));
195+
137196
public static final UDFOperandMetadata NUMERIC_NUMERIC_NUMERIC_NUMERIC_NUMERIC =
138197
UDFOperandMetadata.wrap(
139198
OperandTypes.family(

core/src/main/java/org/opensearch/sql/calcite/utils/binning/BinnableField.java

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,7 @@
1111
import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory;
1212
import org.opensearch.sql.exception.SemanticCheckException;
1313

14-
/**
15-
* Represents a validated field that supports binning operations. The existence of this class
16-
* guarantees that validation has been run - the field is either numeric or time-based.
17-
*
18-
* <p>This design encodes validation in the type system, preventing downstream code from forgetting
19-
* to validate or running validation multiple times.
20-
*/
14+
/** Represents a field that supports binning operations. */
2115
@Getter
2216
public class BinnableField {
2317
private final RexNode fieldExpr;
@@ -27,13 +21,12 @@ public class BinnableField {
2721
private final boolean isNumeric;
2822

2923
/**
30-
* Creates a validated BinnableField. Throws SemanticCheckException if the field is neither
31-
* numeric nor time-based.
24+
* Creates a BinnableField. Validates that the field type is compatible with binning operations.
3225
*
3326
* @param fieldExpr The Rex expression for the field
3427
* @param fieldType The relational data type of the field
3528
* @param fieldName The name of the field (for error messages)
36-
* @throws SemanticCheckException if the field is neither numeric nor time-based
29+
* @throws SemanticCheckException if the field type is not supported for binning
3730
*/
3831
public BinnableField(RexNode fieldExpr, RelDataType fieldType, String fieldName) {
3932
this.fieldExpr = fieldExpr;
@@ -43,13 +36,10 @@ public BinnableField(RexNode fieldExpr, RelDataType fieldType, String fieldName)
4336
this.isTimeBased = OpenSearchTypeFactory.isTimeBasedType(fieldType);
4437
this.isNumeric = OpenSearchTypeFactory.isNumericType(fieldType);
4538

46-
// Validation: field must be either numeric or time-based
39+
// Reject truly unsupported types (e.g., BOOLEAN, ARRAY, MAP)
4740
if (!isNumeric && !isTimeBased) {
4841
throw new SemanticCheckException(
49-
String.format(
50-
"Cannot apply binning: field '%s' is non-numeric and not time-related, expected"
51-
+ " numeric or time-related type",
52-
fieldName));
42+
String.format("Cannot apply binning to field '%s': unsupported type", fieldName));
5343
}
5444
}
5545

0 commit comments

Comments
 (0)