Support UDT for BINARY #3549
Support UDT for BINARY #3549
Conversation
Signed-off-by: Heng Qian <[email protected]>
qianheng-aws
requested review from
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
mengweieric,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto,
GumpacG,
ykmr1224,
LantaoJin and
noCharger
as code owners
Signed-off-by: Heng Qian <[email protected]>
| @@ -60,6 +61,21 @@ public void test_nonnumeric_data_types() throws IOException { | |||
| schema("object_value", "struct"), | |||
| schema("nested_value", "array"), | |||
| schema("geo_point_value", "geo_point")); | |||
| verifyDataRows( | |||
There was a problem hiding this comment.
Could you change verifySchema to verifySchemaInOrder and verifyDataRows to verifyDataRowsInOrder for this test?
| @@ -51,6 +51,8 @@ public enum ExprCoreType implements ExprType { | |||
| /** Geometry. Only support point now. */ | |||
| GEO_POINT(UNDEFINED), | |||
|
|
|||
| BINARY(UNDEFINED), | |||
There was a problem hiding this comment.
Does Calcite impl required ExprCoreType mapping? Should we consider deprecated ExprCoreType in future?
There was a problem hiding this comment.
For the current implementation it requires, because we will always populate the final results with ExprValue transformed from Calcite's results.
| new JSONObject("{\"last\": \"Dale\", \"first\": \"Dale\"}"), | ||
| "keyword", | ||
| new JSONObject("{\"lon\": 74, \"lat\": 40.71}"), | ||
| "U29tZSBiaW5hcnkgYmxvYg==")); |
There was a problem hiding this comment.
what is current value without this PR?
There was a problem hiding this comment.
This is the value V2 returned.
For Calcite, it will throw exception. But this test was ignored in Calcite's IT previously so CI can pass.
There was a problem hiding this comment.
The fallback reason in Calcite's IT is outdated actually, we already have supported IP type. It's true reason is the incompatible BIANRY type.
Signed-off-by: Heng Qian <[email protected]>
# Conflicts: # integ-test/src/test/java/org/opensearch/sql/calcite/remote/nonfallback/NonFallbackCalciteDataTypeIT.java
Signed-off-by: Heng Qian <[email protected]>
Description
We previous transform OpenSearch binary type to Calcite's SqlTypeName.BINARY. However, OpenSearch stores binary value as string while Calcite stores it as ByteString, the gap will lead to execution error if transforming directly.
This PR fixes it by viewing our OpenSearch binary type as a new UDT, which maps to SqlTypeName.VARCHAR
Related Issues
Resolves #3548
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.