alex-plekhanov commented on code in PR #12037:
URL: https://github.com/apache/ignite/pull/12037#discussion_r2152638395


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/sql/fun/IgniteStdSqlOperatorTable.java:
##########
@@ -226,7 +226,11 @@ public IgniteStdSqlOperatorTable() {
         register(SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE);
         register(SqlStdOperatorTable.NEGATED_POSIX_REGEX_CASE_INSENSITIVE);
         register(SqlStdOperatorTable.NEGATED_POSIX_REGEX_CASE_SENSITIVE);
-        register(SqlLibraryOperators.REGEXP_REPLACE);
+        register(SqlLibraryOperators.REGEXP_REPLACE_2);

Review Comment:
   We only supported before:
   `REGEXP_REPLACE(STRING, STRING, STRING, [INT, [INT, [STRING]]])` (Oracle 
syntax)
   This syntax corresponds to operators: REGEXP_REPLACE_3, REGEXP_REPLACE_4, 
REGEXP_REPLACE_5_ORACLE, REGEXP_REPLACE_6
   Let's keep it for compatibility. We don't need REGEXP_REPLACE_2 - it's some 
REDSHIFT syntax.
   
   Corresponding RexImplTable change:
   ```
           defineReflective(REGEXP_REPLACE_3,
               BuiltInMethod.REGEXP_REPLACE3.method,
               BuiltInMethod.REGEXP_REPLACE4.method,
               BuiltInMethod.REGEXP_REPLACE5_OCCURRENCE.method,
               BuiltInMethod.REGEXP_REPLACE6.method);
   ```
   Let's also add one test (only one, no more) for each new operator (4-6 
arguments) to `StdSqlOperatorsTest`, to check that each operator is correctly 
binded to implementator.
   



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java:
##########
@@ -58,8 +58,24 @@ public IgniteRexBuilder(RelDataTypeFactory typeFactory) {
                 return super.makeLiteral(bd, type, type.getSqlTypeName());
             }
 
-            if (TypeUtils.hasScale(type) && SqlTypeUtil.isNumeric(type))
-                return super.makeLiteral(bd.setScale(type.getScale(), 
RoundingMode.HALF_UP), type, typeName);
+            if (SqlTypeUtil.isNumeric(type)) {
+                boolean dfltDecimal = SqlTypeName.DECIMAL == 
type.getSqlTypeName() && bd.scale() > 0
+                    && 
typeFactory.getTypeSystem().getDefaultScale(SqlTypeName.DECIMAL) == 
type.getScale()
+                    && 
typeFactory.getTypeSystem().getDefaultPrecision(SqlTypeName.DECIMAL) == 
type.getPrecision();
+
+                // Keeps scaled values for literals like DECIMAL (converted to 
DECIMAL(32676, 0)) like in Postgres,
+                // keeping actual scale. Example: 2::FLOAT is "2", not "2.0".
+                if (dfltDecimal) {
+                    int precision = Math.max(bd.precision(), bd.scale());

Review Comment:
   TYPEOF(0.1) returns DECIMAL(2, 1)
   TYPEOF(0.1::DECIMAL) returns DECIMAL(1, 1)
   TYPEOF(0::DECIMAL) returns DECIMAL(32767, 0)



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to