Beam-2383 Round function#3326
Conversation
|
@xumingmin please review this PR #3326 |
|
@xumingming can you take a peak here? |
takidau
left a comment
takidau
left a comment
There was a problem hiding this comment.
Thanks a lot, some initial comments.
| return result; | ||
| } | ||
|
|
||
| private int roundInt(int v1, int v2) { |
There was a problem hiding this comment.
I'm not sure I understand what purpose these round* methods serve. They obscure the fact that SqlFunctions.sround is the same (overloaded) method being used in all cases, but don't really seem to provide much. Do they just exist to have shorter names?
There was a problem hiding this comment.
@takidau they are to serve shorter names you got it correct
| } | ||
|
|
||
| @Test public void testRoundFunction() { | ||
| // operands are of type long, int, tinyint |
There was a problem hiding this comment.
This comment doesn't make sense to me. The operands immediately following are doubles.
There was a problem hiding this comment.
@takidau I will correct it to make it more meaningfull
| operands.add(BeamSqlPrimitive.of(SqlTypeName.DOUBLE, 2.0)); | ||
| operands.add(BeamSqlPrimitive.of(SqlTypeName.DOUBLE, 4.0)); | ||
| assertEquals(2.0, new BeamSqlRoundExpression(operands).evaluate(record).getValue()); | ||
| // operands are of type decimal, double, big decimal |
There was a problem hiding this comment.
@takidau I will correct it to make it more meaningfull
| operands.add(ref0); | ||
| operands.add(BeamSqlPrimitive.of(SqlTypeName.BIGINT, 2L)); | ||
|
|
||
| assertEquals(1234567L, new BeamSqlRoundExpression(operands).evaluate(record).getValue()); |
There was a problem hiding this comment.
I'm confused by this one. Where is 1234567 coming from?
There was a problem hiding this comment.
@takidau this refers to BeamSqlInputRefExpression which means first index value we are referencing. Lets suppose I have one row which represents a record with 4 columns
record.addField(0, 1234567L);
record.addField(1, 0);
record.addField(2, 8.9);
record.addField(3, 1234567L);now BeamSqlInputRefExpression helps in referencing a column :
BeamSqlInputRefExpression ref0 = new BeamSqlInputRefExpression(SqlTypeName.BIGINT, 0);public class BeamSQLFnExecutorTestBase {
public static RexBuilder rexBuilder = new RexBuilder(BeamQueryPlanner.TYPE_FACTORY);
public static RelOptCluster cluster = RelOptCluster.create(new VolcanoPlanner(), rexBuilder);
public static final JavaTypeFactory TYPE_FACTORY = new JavaTypeFactoryImpl(
RelDataTypeSystem.DEFAULT);
public static RelDataType relDataType;
public static BeamSQLRecordType beamRecordType;
public static BeamSQLRow record;
public static RelBuilder relBuilder;
@BeforeClass
public static void prepare() {
relDataType = TYPE_FACTORY.builder()
.add("order_id", SqlTypeName.BIGINT)
.add("site_id", SqlTypeName.INTEGER)
.add("price", SqlTypeName.DOUBLE)
.add("order_time", SqlTypeName.BIGINT).build();
beamRecordType = BeamSQLRecordType.from(relDataType);
record = new BeamSQLRow(beamRecordType);
record.addField(0, 1234567L);
record.addField(1, 0);
record.addField(2, 8.9);
record.addField(3, 1234567L);
SchemaPlus schema = Frameworks.createRootSchema(true);
final List<RelTraitDef> traitDefs = new ArrayList<RelTraitDef>();
traitDefs.add(ConventionTraitDef.INSTANCE);
traitDefs.add(RelCollationTraitDef.INSTANCE);
FrameworkConfig config = Frameworks.newConfigBuilder()
.parserConfig(SqlParser.configBuilder().setLex(Lex.MYSQL).build()).defaultSchema(schema)
.traitDefs(traitDefs).context(Contexts.EMPTY_CONTEXT).ruleSets(BeamRuleSets.getRuleSets())
.costFactory(null).typeSystem(BeamRelDataTypeSystem.BEAM_REL_DATATYPE_SYSTEM).build();
relBuilder = RelBuilder.create(config);
}
}
|
@takidau I have made changes in comments. |
|
To clarify, are you wanting to pull this change before #3336, or are you wanting to submit POWER and ROUND together with just that new PR? I think this one is probably ready to go from my perspective. |
|
Great, I'll go ahead and pull this one then. LGTM. |
|
@takidau thanks!! 👍 |
|
Merged on branch DSL_SQL. I updated to include the SQL → Sql renames as part of it. Feel free to close this PR. Thanks! |
5 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify.<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.