[GitHub] [calcite] danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision exc

2020-03-25 Thread GitBox
danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r398303679
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
 ##
 @@ -1015,17 +1015,23 @@ private static boolean flattenFields(
* @param type type descriptor
* @param charSetName  charSet name
* @param maxPrecision The max allowed precision.
+   * @param allowsPrecisionUnspecified whether allows precision not specified
+   * @param defaultPrecision The default precision.
* @return corresponding parse representation
*/
   public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
-  String charSetName, int maxPrecision) {
+  String charSetName, int maxPrecision,
+  boolean allowsPrecisionUnspecified, int defaultPrecision) {
 SqlTypeName typeName = type.getSqlTypeName();
 
 Review comment:
   I would suggest to replace `allowsPrecisionUnspecified` and 
`defaultPrecision` with a single `explicitPrecision`, and if a 
`explicitPrecision` is specified, use it directly.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision exc

2020-03-25 Thread GitBox
danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r398303679
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
 ##
 @@ -1015,17 +1015,23 @@ private static boolean flattenFields(
* @param type type descriptor
* @param charSetName  charSet name
* @param maxPrecision The max allowed precision.
+   * @param allowsPrecisionUnspecified whether allows precision not specified
+   * @param defaultPrecision The default precision.
* @return corresponding parse representation
*/
   public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
-  String charSetName, int maxPrecision) {
+  String charSetName, int maxPrecision,
+  boolean allowsPrecisionUnspecified, int defaultPrecision) {
 SqlTypeName typeName = type.getSqlTypeName();
 
 Review comment:
   I would suggest to replace `allowsPrecisionUnspecified` and 
`defaultPrecision` with a single `explicitPrecision`, and if a 
`explicitPrecision` is specified, use it directly. Leave out the complex 
`allowsPrecisionUnspecified` decision to the invoker.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision exc

2020-03-19 Thread GitBox
danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r395442993
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlDialect.java
 ##
 @@ -772,16 +773,26 @@ public boolean supportsDataType(RelDataType type) {
 return true;
   }
 
- /** Returns SqlNode for type in "cast(column as type)", which might be
+  public boolean allowTypeWithUnspecifiedPrecision(RelDataType type) {
+return true;
+  }
+
+  /** Returns SqlNode for type in "cast(column as type)", which might be
   * different between databases by type name, precision etc. */
   public SqlNode getCastSpec(RelDataType type) {
 if (type instanceof BasicSqlType) {
   int maxPrecision = -1;
   switch (type.getSqlTypeName()) {
   case VARCHAR:
+  case CHAR:
 // if needed, adjust varchar length to max length supported by the 
system
 maxPrecision = getTypeSystem().getMaxPrecision(type.getSqlTypeName());
   }
+  if (type.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) {
+type = allowTypeWithUnspecifiedPrecision(type)
+? type
+: new 
SqlTypeFactoryImpl(getTypeSystem()).createSqlType(type.getSqlTypeName());
 
 Review comment:
   Instead of create a new type, i would prefer to let the 
`SqlTypeUtil.convertTypeToSpec(type, charSet, maxPrecision)` support a explicit 
precision parameter.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision exc

2020-03-19 Thread GitBox
danny0405 commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r395442183
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlDialect.java
 ##
 @@ -772,16 +773,26 @@ public boolean supportsDataType(RelDataType type) {
 return true;
   }
 
- /** Returns SqlNode for type in "cast(column as type)", which might be
+  public boolean allowTypeWithUnspecifiedPrecision(RelDataType type) {
+return true;
+  }
 
 Review comment:
   Can we give a document to this method ? `allowTypeWithUnspecifiedPrecision` 
-> `allowsTypeWithUnspecifiedPrecision`


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services