Abacn commented on code in PR #24013:
URL: https://github.com/apache/beam/pull/24013#discussion_r1015893980


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/utils/CalciteUtils.java:
##########
@@ -109,7 +99,7 @@ public static boolean isStringType(FieldType fieldType) {
   public static final FieldType BOOLEAN = FieldType.BOOLEAN;
   public static final FieldType VARBINARY = FieldType.BYTES;
   public static final FieldType VARCHAR = FieldType.STRING;
-  public static final FieldType CHAR = FieldType.logicalType(new CharType());
+  public static final FieldType CHAR = FieldType.STRING;

Review Comment:
   Yeah, I agree. The "breaking change" I mentioned here is the removal of a 
public field.
   
   We have the same situation for fixed binary:
   
https://github.com/apache/beam/blob/60e465e40d413ea6fb8c113ff5619c30be5864b0/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/utils/CalciteUtils.java#L147
   and
   
https://github.com/apache/beam/blob/60e465e40d413ea6fb8c113ff5619c30be5864b0/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/utils/CalciteUtils.java#L100
   makes BINARY the same as BYTES in CalciteUtils currently.
   
   A full support for BINARY/VARBINARY/CHAR/VARCHAR with specified limit 
involves taking Calcite logical type's precision parameter (currently it is 
omitted) and pass it to FixedBinary/VariableBinary/FixedString/VariableString 
constructors. We cannot have constant logical type instance for these types 
because they require an argument in constructor. The full support for these 
types is left as future work. For now this change just unblock the use case 
that mentioned in https://issues.apache.org/jira/browse/BEAM-12323.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to