IGNITE-28801 Calcite. Bump calcite dependency version to 1.42#13274
IGNITE-28801 Calcite. Bump calcite dependency version to 1.42#13274zstan wants to merge 8 commits into
Conversation
| BuiltInMethod.BIG_DECIMAL_NEGATE.getMethodName()); | ||
| defineUnary(UNARY_PLUS, UnaryPlus, NullPolicy.STRICT, null); | ||
| // checked arithmetic | ||
| defineBinary(CHECKED_PLUS, AddChecked, NullPolicy.STRICT, "checkedPlus"); |
There was a problem hiding this comment.
due to categorical CHECED type without conformance flag check
https://issues.apache.org/jira/browse/CALCITE-7443
| // For checked arithmetic call the method. | ||
| if (CHECKED_OPERATORS.contains(op)) | ||
| return Expressions.call(SqlFunctions.class, backupMethodName, argValueList); | ||
| //if (CHECKED_OPERATORS.contains(op)) |
There was a problem hiding this comment.
| public static Expression makeBinary(ExpressionType binaryType, Expression left, Expression right) { | ||
| switch (binaryType) { | ||
| case Add: | ||
| case Add, AddChecked: |
There was a problem hiding this comment.
check comment near, due to
https://issues.apache.org/jira/browse/CALCITE-7443
| ConstantExpression formatExpr; | ||
|
|
||
| // Check for FORMAT clause if second operand is available in RexCall. | ||
| if (call.operandCount() == 2) { |
There was a problem hiding this comment.
just runtime code alignment
| "BITXOR", | ||
| SqlKind.OTHER_FUNCTION, | ||
| ReturnTypes.LEAST_RESTRICTIVE, | ||
| ReturnTypes.BIGINT_NULLABLE, |
There was a problem hiding this comment.
after this
https://issues.apache.org/jira/browse/CALCITE-6731
return type resolved as Object which breaks futrher scalar compilation
There was a problem hiding this comment.
Looks weird. Can we write new own SqlReturnTypeInference to derive correct data type as workaround?
| /** {@inheritDoc} */ | ||
| @Override public int getMaxNumericScale() { | ||
| return Short.MAX_VALUE; | ||
| @Override public int getMaxScale(SqlTypeName typeName) { |
There was a problem hiding this comment.
getMaxNumericScale\getMaxNumericPrecision - deprecated and moved to final notation
zstan
force-pushed
the
ignite-28801
branch
3 times, most recently
from
35bc26e to
bf6ecc6
Compare
| files="ConcurrentLinkedHashMap\.java"/> | ||
|
|
||
| <suppress checks=".*" | ||
| files="RexImpTable\.java|RexToLixTranslator\.java"/> |
There was a problem hiding this comment.
Let`s remove checks from calcite runtime files, it`s hard to compare with base branch otherwize
There was a problem hiding this comment.
Currently only limited lines have been added, but already error (unused import) was introduced. I think we should check these files too, at least for now.
| import org.apache.calcite.rex.RexCall; | ||
| import org.apache.calcite.rex.RexLiteral; | ||
| import org.apache.calcite.rex.RexNode; | ||
| import org.apache.calcite.runtime.FlatLists; |
There was a problem hiding this comment.
Unused import.
We should not exclude this file from checkstyle to prevent such errors
| final RexToLixTranslator translator, | ||
| final RexCall call, | ||
| final List<Expression> argValueList) { | ||
| List<Expression> argValueList) { |
There was a problem hiding this comment.
Looks like it still final, why final was removed?
| } else if (expressionType == NegateChecked && null != backupMethodName) { | ||
| e = Expressions.call(SqlFunctions.class, backupMethodName, argValueList); |
There was a problem hiding this comment.
Let's keep codestyle and just add these two lines
|
|
||
| if (!allowNulls.get(p)) { | ||
| collsAllowNullsBuilder.clear(c); | ||
| if (!nullExclusions.get(p)) { |
There was a problem hiding this comment.
nullExclusions and allowNulls have inverse logic, why here not allowNulls changed to not null exclusion? Shouldn't we remove not?
| "BITXOR", | ||
| SqlKind.OTHER_FUNCTION, | ||
| ReturnTypes.LEAST_RESTRICTIVE, | ||
| ReturnTypes.BIGINT_NULLABLE, |
There was a problem hiding this comment.
Looks weird. Can we write new own SqlReturnTypeInference to derive correct data type as workaround?
| * mvn generate-sources -pl :ignite-calcite | ||
| */ | ||
|
|
||
| package org.apache.ignite.internal.processors.query.calcite.sql.generated; |
| .and(s -> "=($0, $cor0.ID)".equals(s.condition().toString())) | ||
| .and(hasChildThat(isInstanceOf(IgniteTableScan.class) | ||
| .and(t -> "<($t0, 50)".equals(t.condition().toString())))))); | ||
| .and(t -> "<(CAST($t0):INTEGER, 50)".equals(t.condition().toString())))))); |
There was a problem hiding this comment.
Let's change field definition from Integer.class to SqlTypeName.INTEGER instead
| Expression operand) { | ||
| Expression operand, | ||
| boolean safe, | ||
| ConstantExpression format) { |
There was a problem hiding this comment.
If we allow format now (have format processing code in CastImplementor) why do we ignore it here?
| files="ConcurrentLinkedHashMap\.java"/> | ||
|
|
||
| <suppress checks=".*" | ||
| files="RexImpTable\.java|RexToLixTranslator\.java"/> |
There was a problem hiding this comment.
Currently only limited lines have been added, but already error (unused import) was introduced. I think we should check these files too, at least for now.
2 participants
https://issues.apache.org/jira/browse/IGNITE-28801