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