Skip to content

[CALCITE-3380] implement power method in Expressions. - #1480

Closed
xy2953396112 wants to merge 2 commits into
apache:masterfrom
xy2953396112:power
Closed

[CALCITE-3380] implement power method in Expressions.#1480
xy2953396112 wants to merge 2 commits into
apache:masterfrom
xy2953396112:power

Conversation

@xy2953396112

@xy2953396112 xy2953396112 commented Sep 30, 2019

Copy link
Copy Markdown
Contributor

implement power method in Expressions.
before:
Extensions.todo();

@julianhyde

Copy link
Copy Markdown
Contributor

I don't think we need this method. linq4j is tied to the java language. There is no power operator in java, therefore people should not be using the Expressions.power function, but instead make a method call.

Rather than Expressions.power(a, b) use Expressions.call(null, Math.class.getMethod("power"), a, b). Then I think the evaluate will be able to evaluate it without any extra work.

@xy2953396112

Copy link
Copy Markdown
Contributor Author

I don't think we need this method. linq4j is tied to the java language. There is no power operator in java, therefore people should not be using the Expressions.power function, but instead make a method call.

Rather than Expressions.power(a, b) use Expressions.call(null, Math.class.getMethod("power"), a, b). Then I think the evaluate will be able to evaluate it without any extra work.

It is more convenient for users to use Expressions.power(a, b)

@danny0405 danny0405 added the returned-with-feedback There are review comments (in JIRA and/or in GitHub) to be implemented before merging the PR label Oct 17, 2019
@julianhyde

Copy link
Copy Markdown
Contributor

I agree that it is a bit more convenient. But it goes against the philosophy of this class and package. We should not do it.

@danny0405

Copy link
Copy Markdown
Contributor

Close because it is not needed.

@danny0405 danny0405 closed this Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

returned-with-feedback There are review comments (in JIRA and/or in GitHub) to be implemented before merging the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants