[BEAM-2563] Add integration test for math operators #3569
Conversation
| @@ -280,6 +283,7 @@ static BeamSqlExpression buildExpression(RexNode rexNode) { | |||
| case "SIGN": | |||
| ret = new BeamSqlSignExpression(subExps); | |||
| break; | |||
| case "SQRT": //SQRT is converted as POWER(i, 0.5) | |||
There was a problem hiding this comment.
if SQRT is converted to POWER, then we don't even need the case statement, right?
There was a problem hiding this comment.
right, it's not necessary here.
| @@ -37,7 +37,13 @@ public BeamSqlSignExpression(List<BeamSqlExpression> operands) { | |||
| BeamSqlPrimitive result = null; | |||
| switch (op.getOutputType()) { | |||
There was a problem hiding this comment.
let's specify the outputType for BeamSqlSignExpression in the constructor:
public BeamSqlSignExpression(List<BeamSqlExpression> operands) {
super(operands);
}to
public BeamSqlSignExpression(List<BeamSqlExpression> operands) {
super(operands, op(0).getOutputType());
}There was a problem hiding this comment.
sound right, need to rewrite the constructors for all built-in MATH functions, otherwise it fails with nested methods like SIGN(ABS($1)).
|
retest this please |
|
Retest this please |
1 similar comment
|
Retest this please |
|
R + @JingsongLi, I'll take a quick look. |
|
@JingsongLi There's another blocked PR #3552, not sue if you can take it, or need to wait for Tyler? |
|
It is best to wait for Tyler to look at your fix after his review. |
Misc: 1. no SQRT in Calcite, converted to POWER; 2. add DECIMAL in BeamSqlDslBase; 3. fix error in BeamSqlRoundExpression; 4. fix error in BeamSqlSignExpression;
|
@JingsongLi the tests are all here, please review it when you've time. |
|
retest this please |
|
LGTM once Travis gives the green. |
|
retest this please |
|
Merged, feel free to close. |
|
Thank you @xumingming @JingsongLi |
Add integration test for built-in MATH functions,
Misc: