Skip to content

Commit 6e43491

Browse files
committed
Fix issue 3339
Signed-off-by: Peng Huo <penghuo@gmail.com>
1 parent 28275b8 commit 6e43491

File tree

6 files changed

+154
-13
lines changed

6 files changed

+154
-13
lines changed

integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ public void test_numeric_data_types() throws IOException {
3232
JSONObject result = executeQuery(String.format("source=%s", TEST_INDEX_DATATYPE_NUMERIC));
3333
verifySchema(
3434
result,
35-
schema("long_number", "long"),
36-
schema("integer_number", "integer"),
37-
schema("short_number", "short"),
38-
schema("byte_number", "byte"),
35+
schema("long_number", "bigint"),
36+
schema("integer_number", "int"),
37+
schema("short_number", "smallint"),
38+
schema("byte_number", "tinyint"),
3939
schema("double_number", "double"),
4040
schema("float_number", "float"),
4141
schema("half_float_number", "float"),
@@ -73,10 +73,10 @@ public void test_long_integer_data_type() throws IOException {
7373
TEST_INDEX_DATATYPE_NUMERIC));
7474
verifySchema(
7575
result,
76-
schema("int1", "integer"),
77-
schema("int2", "integer"),
78-
schema("long1", "long"),
79-
schema("long2", "long"));
76+
schema("int1", "int"),
77+
schema("int2", "int"),
78+
schema("long1", "bigint"),
79+
schema("long2", "bigint"));
8080
}
8181

8282
@Test
@@ -86,6 +86,6 @@ public void test_alias_data_type() throws IOException {
8686
String.format(
8787
"source=%s | where alias_col > 1 " + "| fields original_col, alias_col ",
8888
TEST_INDEX_ALIAS));
89-
verifySchema(result, schema("original_col", "integer"), schema("alias_col", "integer"));
89+
verifySchema(result, schema("original_col", "int"), schema("alias_col", "int"));
9090
}
9191
}

plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.opensearch.sql.ppl.PPLService;
3434
import org.opensearch.sql.ppl.domain.PPLQueryRequest;
3535
import org.opensearch.sql.protocol.response.QueryResult;
36+
import org.opensearch.sql.protocol.response.datatype.PPLResponseDataType;
3637
import org.opensearch.sql.protocol.response.format.CsvResponseFormatter;
3738
import org.opensearch.sql.protocol.response.format.Format;
3839
import org.opensearch.sql.protocol.response.format.JsonResponseFormatter;
@@ -160,7 +161,8 @@ private ResponseListener<ExecutionEngine.QueryResponse> createListener(
160161
public void onResponse(ExecutionEngine.QueryResponse response) {
161162
String responseContent =
162163
formatter.format(
163-
new QueryResult(response.getSchema(), response.getResults(), response.getCursor()));
164+
new QueryResult(response.getSchema(), response.getResults(), response.getCursor()
165+
, PPLResponseDataType.getInstance()));
164166
listener.onResponse(new TransportPPLQueryResponse(responseContent));
165167
}
166168

protocol/src/main/java/org/opensearch/sql/protocol/response/QueryResult.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717
import org.opensearch.sql.executor.ExecutionEngine;
1818
import org.opensearch.sql.executor.ExecutionEngine.Schema.Column;
1919
import org.opensearch.sql.executor.pagination.Cursor;
20+
import org.opensearch.sql.protocol.response.datatype.ResponseDataType;
2021

2122
/**
2223
* Query response that encapsulates query results and isolate {@link ExprValue} related from
2324
* formatter implementation.
2425
*/
25-
@RequiredArgsConstructor
2626
public class QueryResult implements Iterable<Object[]> {
2727

2828
@Getter private final ExecutionEngine.Schema schema;
@@ -32,8 +32,23 @@ public class QueryResult implements Iterable<Object[]> {
3232

3333
@Getter private final Cursor cursor;
3434

35+
private final ResponseDataType responseDataType;
36+
3537
public QueryResult(ExecutionEngine.Schema schema, Collection<ExprValue> exprValues) {
36-
this(schema, exprValues, Cursor.None);
38+
this(schema, exprValues, Cursor.None, ResponseDataType.DEFAULT);
39+
}
40+
41+
public QueryResult(ExecutionEngine.Schema schema, Collection<ExprValue> exprValues, Cursor cursor) {
42+
this(schema, exprValues, cursor, ResponseDataType.DEFAULT);
43+
}
44+
45+
public QueryResult(ExecutionEngine.Schema schema, Collection<ExprValue> exprValues,
46+
Cursor cursor,
47+
ResponseDataType responseDataType) {
48+
this.schema = schema;
49+
this.exprValues = exprValues;
50+
this.cursor = cursor;
51+
this.responseDataType = responseDataType;
3752
}
3853

3954
/**
@@ -59,7 +74,7 @@ public Map<String, String> columnNameTypes() {
5974
column ->
6075
colNameTypes.put(
6176
getColumnName(column),
62-
column.getExprType().typeName().toLowerCase(Locale.ROOT)));
77+
responseDataType.typeName(column.getExprType()).toLowerCase(Locale.ROOT)));
6378
return colNameTypes;
6479
}
6580

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.protocol.response.datatype;
7+
8+
import java.util.HashMap;
9+
import java.util.Map;
10+
import org.opensearch.sql.data.type.ExprCoreType;
11+
import org.opensearch.sql.data.type.ExprType;
12+
13+
/**
14+
* Singleton implementation of {@code ResponseDataType} for PPL.
15+
* <p>
16+
* This class maps expression types to their corresponding PPL types.
17+
* The mapping includes BYTE to "tinyint", SHORT to "smallint",
18+
* INTEGER to "int", and LONG to "bigint".
19+
* </p>
20+
*/
21+
public class PPLResponseDataType implements ResponseDataType{
22+
23+
private static final PPLResponseDataType INSTANCE = new PPLResponseDataType();
24+
25+
private static Map<ExprType, String> exprTypeToPPLType = new HashMap<>();
26+
27+
static {
28+
exprTypeToPPLType.put(ExprCoreType.BYTE, "tinyint");
29+
exprTypeToPPLType.put(ExprCoreType.SHORT, "smallint");
30+
exprTypeToPPLType.put(ExprCoreType.INTEGER, "int");
31+
exprTypeToPPLType.put(ExprCoreType.LONG, "bigint");
32+
}
33+
34+
private PPLResponseDataType() {}
35+
36+
public static PPLResponseDataType getInstance() {
37+
return INSTANCE;
38+
}
39+
40+
/**
41+
* Returns the corresponding PPL type name for the given expression type.
42+
* If the expression type is not mapped, it returns the default type name.
43+
*
44+
* @param exprType the expression type.
45+
* @return the PPL type name associated with the expression type, or the default type name.
46+
*/
47+
@Override
48+
public String typeName(ExprType exprType) {
49+
return exprTypeToPPLType.getOrDefault(exprType, exprType.typeName());
50+
}
51+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.protocol.response.datatype;
7+
8+
import org.opensearch.sql.data.type.ExprType;
9+
10+
public interface ResponseDataType {
11+
12+
ResponseDataType DEFAULT = new ResponseDataType() {};
13+
14+
default String typeName(ExprType exprType) {
15+
return exprType.typeName();
16+
}
17+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.protocol.response.datatype;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertSame;
10+
11+
import org.junit.jupiter.api.Test;
12+
import org.opensearch.sql.data.type.ExprCoreType;
13+
import org.opensearch.sql.data.type.ExprType;
14+
15+
class PPLResponseDataTypeTest {
16+
/**
17+
* Tests that the same instance is returned from the singleton.
18+
*/
19+
@Test
20+
public void testSingletonInstance() {
21+
PPLResponseDataType instance1 = PPLResponseDataType.getInstance();
22+
PPLResponseDataType instance2 = PPLResponseDataType.getInstance();
23+
assertSame(instance1, instance2, "Expected both instances to be the same (singleton).");
24+
}
25+
26+
/**
27+
* Tests that known expression types return the correct PPL type names.
28+
*/
29+
@Test
30+
public void testTypeNameMapping() {
31+
PPLResponseDataType instance = PPLResponseDataType.getInstance();
32+
assertEquals("tinyint", instance.typeName(ExprCoreType.BYTE), "BYTE should map to tinyint.");
33+
assertEquals("smallint", instance.typeName(ExprCoreType.SHORT), "SHORT should map to smallint.");
34+
assertEquals("int", instance.typeName(ExprCoreType.INTEGER), "INTEGER should map to int.");
35+
assertEquals("bigint", instance.typeName(ExprCoreType.LONG), "LONG should map to bigint.");
36+
}
37+
38+
/**
39+
* Tests that an unmapped expression type returns its default type name.
40+
*/
41+
@Test
42+
public void testTypeNameDefault() {
43+
PPLResponseDataType instance = PPLResponseDataType.getInstance();
44+
45+
// Create a dummy expression type not in the mapping.
46+
ExprType dummyExprType = new ExprType() {
47+
@Override
48+
public String typeName() {
49+
return "defaultType";
50+
}
51+
};
52+
53+
assertEquals("defaultType", instance.typeName(dummyExprType),
54+
"Unmapped expression type should return its default type name.");
55+
}
56+
}

0 commit comments

Comments
 (0)