Skip to content

Commit e135281

Browse files
vinaykpudvinaykpud
committed
Added coercion rules and placeholder UDF to handle VARBINARY
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
1 parent c5ae56f commit e135281

8 files changed

Lines changed: 207 additions & 2 deletions

File tree

‎core/src/main/java/org/opensearch/sql/calcite/ExtendedRexBuilder.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,12 @@ else if ((SqlTypeUtil.isApproximateNumeric(sourceType) || SqlTypeUtil.isDecimal(
183183
// NUMBER_TO_STRING uses java's built-in method to get the string representation of a number
184184
return makeCall(type, PPLBuiltinOperators.NUMBER_TO_STRING, List.of(exp));
185185
}
186+
// VARCHAR → VARBINARY for ip/binary fields. Emit BINARY(varchar) as a placeholder
187+
// RexCall the analytics backend adapter rewrites into a VARBINARY literal.
188+
else if (sqlType == SqlTypeName.VARBINARY
189+
&& sourceType.getSqlTypeName() == SqlTypeName.VARCHAR) {
190+
return makeCall(type, PPLBuiltinOperators.BINARY, List.of(exp));
191+
}
186192
return super.makeCast(pos, type, exp, matchNullability, safe, format);
187193
}
188194
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ public static RelDataType convertExprTypeToRelDataType(ExprType fieldType, boole
174174
return TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIME, nullable);
175175
case TIMESTAMP:
176176
return TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIMESTAMP, nullable);
177+
case BINARY:
178+
return TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY, nullable);
177179
case ARRAY:
178180
return TYPE_FACTORY.createArrayType(
179181
TYPE_FACTORY.createSqlType(SqlTypeName.ANY, nullable), -1);
@@ -225,6 +227,7 @@ public static ExprType convertSqlTypeNameToExprType(SqlTypeName sqlTypeName) {
225227
case FLOAT, REAL -> FLOAT;
226228
case DOUBLE, DECIMAL -> DOUBLE; // TODO the decimal is only used for literal
227229
case CHAR, VARCHAR, MULTISET -> STRING; // call toString() for MULTISET
230+
case VARBINARY, BINARY -> BINARY;
228231
case BOOLEAN -> BOOLEAN;
229232
case DATE -> DATE;
230233
case TIME, TIME_TZ, TIME_WITH_LOCAL_TIME_ZONE -> TIME;

‎core/src/main/java/org/opensearch/sql/expression/function/CoercionUtils.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,12 @@ public static boolean hasString(List<RexNode> rexNodeList) {
181181
(left, right) -> ExprCoreType.TIMESTAMP),
182182
CoercionRule.of(
183183
(left, right) -> hasString(left, right) && hasNumber(left, right),
184-
(left, right) -> ExprCoreType.DOUBLE));
184+
(left, right) -> ExprCoreType.DOUBLE),
185+
// (BINARY, STRING) → BINARY: ip/binary columns compared with string literals.
186+
// Triggers ExtendedRexBuilder.makeCast which wraps the literal with BINARY.
187+
CoercionRule.of(
188+
(left, right) -> hasString(left, right) && hasBinary(left, right),
189+
(left, right) -> ExprCoreType.BINARY));
185190

186191
private static boolean hasString(ExprType left, ExprType right) {
187192
return left == ExprCoreType.STRING || right == ExprCoreType.STRING;
@@ -195,6 +200,10 @@ private static boolean hasBoolean(ExprType left, ExprType right) {
195200
return left == ExprCoreType.BOOLEAN || right == ExprCoreType.BOOLEAN;
196201
}
197202

203+
private static boolean hasBinary(ExprType left, ExprType right) {
204+
return left == ExprCoreType.BINARY || right == ExprCoreType.BINARY;
205+
}
206+
198207
private record CoercionRule(
199208
BiPredicate<ExprType, ExprType> predicate, BinaryOperator<ExprType> resolver) {
200209

‎core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
import org.opensearch.sql.expression.function.udf.condition.EarliestFunction;
8484
import org.opensearch.sql.expression.function.udf.condition.EnhancedCoalesceFunction;
8585
import org.opensearch.sql.expression.function.udf.condition.LatestFunction;
86+
import org.opensearch.sql.expression.function.udf.conversion.BinaryFunction;
8687
import org.opensearch.sql.expression.function.udf.datetime.AddSubDateFunction;
8788
import org.opensearch.sql.expression.function.udf.datetime.CurrentFunction;
8889
import org.opensearch.sql.expression.function.udf.datetime.DateAddSubFunction;
@@ -179,6 +180,9 @@ public class PPLBuiltinOperators extends ReflectiveSqlOperatorTable {
179180
public static final SqlOperator EARLIEST = new EarliestFunction().toUDF("EARLIEST");
180181
public static final SqlOperator LATEST = new LatestFunction().toUDF("LATEST");
181182

183+
// VARBINARY conversion (placeholder for ip/binary fields rewritten by analytics backend adapter)
184+
public static final SqlOperator BINARY = new BinaryFunction().toUDF("BINARY");
185+
182186
// Datetime function
183187
public static final SqlOperator TIMESTAMP = new TimestampFunction().toUDF("TIMESTAMP");
184188
public static final SqlOperator DATE =

‎core/src/main/java/org/opensearch/sql/expression/function/udf/conversion/BinaryFunction.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.expression.function.udf.conversion;
7+
8+
import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY;
9+
10+
import java.util.List;
11+
import org.apache.calcite.adapter.enumerable.NotNullImplementor;
12+
import org.apache.calcite.adapter.enumerable.NullPolicy;
13+
import org.apache.calcite.adapter.enumerable.RexToLixTranslator;
14+
import org.apache.calcite.linq4j.tree.Expression;
15+
import org.apache.calcite.rex.RexCall;
16+
import org.apache.calcite.sql.type.FamilyOperandTypeChecker;
17+
import org.apache.calcite.sql.type.OperandTypes;
18+
import org.apache.calcite.sql.type.ReturnTypes;
19+
import org.apache.calcite.sql.type.SqlReturnTypeInference;
20+
import org.apache.calcite.sql.type.SqlTypeName;
21+
import org.opensearch.sql.expression.function.ImplementorUDF;
22+
import org.opensearch.sql.expression.function.UDFOperandMetadata;
23+
24+
/**
25+
* Placeholder UDF that wraps a VARCHAR literal cast to VARBINARY for ip/binary fields.
26+
*/
27+
public class BinaryFunction extends ImplementorUDF {
28+
29+
private static final SqlReturnTypeInference VARBINARY_FORCE_NULLABLE =
30+
ReturnTypes.explicit(
31+
TYPE_FACTORY.createTypeWithNullability(
32+
TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY), true));
33+
34+
public BinaryFunction() {
35+
super(new PassThroughImplementor(), NullPolicy.STRICT);
36+
}
37+
38+
@Override
39+
public SqlReturnTypeInference getReturnTypeInference() {
40+
return VARBINARY_FORCE_NULLABLE;
41+
}
42+
43+
@Override
44+
public UDFOperandMetadata getOperandMetadata() {
45+
return UDFOperandMetadata.wrap((FamilyOperandTypeChecker) OperandTypes.CHARACTER);
46+
}
47+
48+
public static class PassThroughImplementor implements NotNullImplementor {
49+
@Override
50+
public Expression implement(
51+
RexToLixTranslator translator, RexCall call, List<Expression> translatedOperands) {
52+
return translatedOperands.get(0);
53+
}
54+
}
55+
}

‎core/src/test/java/org/opensearch/sql/calcite/ExtendedRexBuilderTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.calcite;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
10+
import static org.junit.jupiter.api.Assertions.assertNotNull;
11+
import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY;
12+
13+
import org.apache.calcite.rel.type.RelDataType;
14+
import org.apache.calcite.rex.RexBuilder;
15+
import org.apache.calcite.rex.RexCall;
16+
import org.apache.calcite.rex.RexNode;
17+
import org.apache.calcite.sql.type.SqlTypeName;
18+
import org.junit.jupiter.api.Test;
19+
import org.opensearch.sql.expression.function.PPLBuiltinOperators;
20+
21+
class ExtendedRexBuilderTest {
22+
23+
private static final RexBuilder REX_BUILDER = new ExtendedRexBuilder(new RexBuilder(TYPE_FACTORY));
24+
25+
/**
26+
* VARCHAR → VARBINARY casts must be rewritten as a {@code BINARY(varchar)} placeholder
27+
* {@code RexCall}.
28+
*/
29+
@Test
30+
void castVarcharToVarbinaryEmitsBinaryPlaceholder() {
31+
// Use makeInputRef to construct a VARCHAR-typed RexNode reliably. makeLiteral(String) folds
32+
// to CHAR, which would make this test pass-through default cast instead of our placeholder.
33+
RelDataType varchar = TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR);
34+
RexNode varcharRef = REX_BUILDER.makeInputRef(varchar, 0);
35+
RelDataType varbinary = TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY);
36+
37+
RexNode result = REX_BUILDER.makeCast(varbinary, varcharRef);
38+
39+
assertInstanceOf(RexCall.class, result);
40+
RexCall call = (RexCall) result;
41+
assertEquals(PPLBuiltinOperators.BINARY, call.getOperator());
42+
assertEquals("BINARY", call.getOperator().getName());
43+
assertEquals(SqlTypeName.VARBINARY, call.getType().getSqlTypeName());
44+
assertEquals(1, call.getOperands().size());
45+
assertEquals(SqlTypeName.VARCHAR, call.getOperands().get(0).getType().getSqlTypeName());
46+
}
47+
48+
/**
49+
* Casts targeting a SqlTypeName other than VARBINARY must NOT trigger the BINARY rewrite —
50+
* they fall through to Calcite's default cast handling.
51+
*/
52+
@Test
53+
void castVarcharToIntegerDoesNotEmitBinaryPlaceholder() {
54+
RexNode varcharLiteral = REX_BUILDER.makeLiteral("42");
55+
RelDataType integer = TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER);
56+
57+
RexNode result = REX_BUILDER.makeCast(integer, varcharLiteral);
58+
59+
assertNotNull(result);
60+
if (result instanceof RexCall call) {
61+
assertEquals(
62+
"BINARY".equals(call.getOperator().getName()),
63+
false,
64+
"VARCHAR → INTEGER must not emit BINARY placeholder");
65+
}
66+
}
67+
68+
/**
69+
* Casts whose source is not VARCHAR must also fall through. The placeholder is only meant for
70+
* the (VARCHAR → VARBINARY) case where a string IP / base64 literal is being compared against a
71+
* VARBINARY column — non-string sources have well-defined Calcite cast semantics that should
72+
* not be hijacked.
73+
*/
74+
@Test
75+
void castIntegerToVarbinaryDoesNotEmitBinaryPlaceholder() {
76+
RexNode intLiteral = REX_BUILDER.makeExactLiteral(java.math.BigDecimal.ONE);
77+
RelDataType varbinary = TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY);
78+
79+
RexNode result = REX_BUILDER.makeCast(varbinary, intLiteral);
80+
81+
assertNotNull(result);
82+
if (result instanceof RexCall call) {
83+
assertEquals(
84+
"BINARY".equals(call.getOperator().getName()),
85+
false,
86+
"non-VARCHAR → VARBINARY must not emit BINARY placeholder");
87+
}
88+
}
89+
}

‎core/src/test/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactoryTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.junit.jupiter.api.Test;
1818
import org.opensearch.sql.calcite.type.AbstractExprRelDataType;
1919
import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT;
20+
import org.opensearch.sql.data.type.ExprCoreType;
2021

2122
public class OpenSearchTypeFactoryTest {
2223

@@ -126,4 +127,39 @@ public void testLeastRestrictiveDelegatesToSuperForPlainTypes() {
126127
assertNotNull(result);
127128
assertEquals(SqlTypeName.INTEGER, result.getSqlTypeName());
128129
}
130+
131+
@Test
132+
public void testConvertSqlTypeNameVarbinaryToBinaryExprType() {
133+
assertEquals(
134+
ExprCoreType.BINARY, OpenSearchTypeFactory.convertSqlTypeNameToExprType(SqlTypeName.VARBINARY));
135+
}
136+
137+
@Test
138+
public void testConvertSqlTypeNameBinaryToBinaryExprType() {
139+
assertEquals(
140+
ExprCoreType.BINARY, OpenSearchTypeFactory.convertSqlTypeNameToExprType(SqlTypeName.BINARY));
141+
}
142+
143+
@Test
144+
public void testConvertRelDataTypeVarbinaryToBinaryExprType() {
145+
RelDataType varbinary = TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY);
146+
assertEquals(
147+
ExprCoreType.BINARY, OpenSearchTypeFactory.convertRelDataTypeToExprType(varbinary));
148+
}
149+
150+
@Test
151+
public void testConvertExprTypeBinaryToVarbinaryRelDataType() {
152+
RelDataType result = OpenSearchTypeFactory.convertExprTypeToRelDataType(ExprCoreType.BINARY);
153+
assertNotNull(result);
154+
assertEquals(SqlTypeName.VARBINARY, result.getSqlTypeName());
155+
}
156+
157+
@Test
158+
public void testConvertExprTypeBinaryToNullableVarbinary() {
159+
RelDataType result =
160+
OpenSearchTypeFactory.convertExprTypeToRelDataType(ExprCoreType.BINARY, true);
161+
assertNotNull(result);
162+
assertEquals(SqlTypeName.VARBINARY, result.getSqlTypeName());
163+
assertTrue(result.isNullable());
164+
}
129165
}

‎core/src/test/java/org/opensearch/sql/expression/function/CoercionUtilsTest.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.opensearch.sql.expression.function;
77

88
import static org.junit.jupiter.api.Assertions.*;
9+
import static org.opensearch.sql.data.type.ExprCoreType.BINARY;
910
import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
1011
import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE;
1112
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;
@@ -37,7 +38,9 @@ private static Stream<Arguments> commonWidestTypeArguments() {
3738
Arguments.of(STRING, INTEGER, DOUBLE),
3839
Arguments.of(INTEGER, STRING, DOUBLE),
3940
Arguments.of(STRING, DOUBLE, DOUBLE),
40-
Arguments.of(INTEGER, BOOLEAN, null));
41+
Arguments.of(INTEGER, BOOLEAN, null),
42+
Arguments.of(BINARY, STRING, BINARY),
43+
Arguments.of(STRING, BINARY, BINARY));
4144
}
4245

4346
@ParameterizedTest

0 commit comments

Comments
 (0)