You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ongchi (via GitHub)" <gi...@apache.org> on 2023/10/03 14:13:19 UTC
[PR] Add operator section to user guide [arrow-datafusion]
ongchi opened a new pull request, #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #7681
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1746050317
Methods added to `Expr` in this PR:
- Methods already listed in documentation but not implemented:
- bitwise_and
- bitwise_or
- bitwise_xor
- bitwise_shift_left
- bitwise_shift_right
- not
- Additional arithmetic methods introduced in this PR:
- As same as arithmetic functions listed above
I found that there are multiple implementations to do the same thing, e.g., bitwise operations on https://github.com/apache/arrow-datafusion/blob/a64f36d8fbce35cb7146271fb5d9ab98a9f094e6/datafusion/expr/src/expr_fn.rs#L172-L215 and https://github.com/apache/arrow-datafusion/blob/a64f36d8fbce35cb7146271fb5d9ab98a9f094e6/datafusion/expr/src/operator.rs#L313-L355, that should/is identical to each other. Do we have to merge these implementations and place tests in the same place?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346075338
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
+| Operation | Syntax | Method | Function |
+| ------------------------- | -------- | -------------------------- | --------------------------- |
+| Arithmetic addition | `x + y` | `x.add(y)` | `add(x, y)` |
+| Arithmetic subtraction | `x - y` | `x.sub(y)` | `sub(x, y)` |
+| Arithmetic multiplication | `x * y` | `x.mul(y)` | `mul(x, y)` |
+| Arithmetic division | `x / y` | `x.div(y)` | `div(x, y)` |
+| Arithmetic remainder | `x % y` | `x.r#mod(y)` | `r#mod(x, y)` |
+| Arithmetic negation | `-x` | `x.neg()` | `neg(x, y)` |
+| Bitwise AND | `x & y` | `x.bitwise_and(y)` | `bitwise_and(x, y)` |
+| Bitwise OR | `x \| y` | `x.bitwise_or(y)` | `bitwise_or(x, y)` |
+| Bitwise XOR | `x ^ y` | `x.bitwise_xor(y)` | `bitwise_xor(x, y)` |
+| Bitwise left shift | `x << y` | `x.bitwise_shift_left(y)` | `bitwise_shift_left(x, y)` |
+| Bitwise right shift | `x >> y` | `x.bitwise_shift_right(y)` | `bitwise_shift_right(x, y)` |
+| Logical NOT | `!x` | `x.not()` | `not(x)` |
+
+:::{note}
+`mod` is a reserved keyword in Rust, we use a raw identifier to name the `r#mod` function.
+:::
+
+:::{note}
+`!` is a bitwise or logical complement operator in Rust, but it only works as a logical NOT in expression API.
+:::
## Boolean Expressions
-| Function | Notes |
-| -------- | ----------------------------------------- |
-| and | `and(expr1, expr2)` or `expr1.and(expr2)` |
-| or | `or(expr1, expr2)` or `expr1.or(expr2)` |
-| not | `not(expr)` or `expr.not()` |
-
-## Bitwise expressions
+| Function | Method | Explanation |
+| ----------- | ---------- | ----------- |
+| `and(x, y)` | `x.and(y)` | Logical AND |
+| `or(x, y)` | `x.or(y)` | Logical OR |
+| `not(x)` | `x.not()` | Logical NOT |
-| Function | Notes |
-| ------------------- | ------------------------------------------------------------------------- |
-| bitwise_and | `bitwise_and(expr1, expr2)` or `expr1.bitwise_and(expr2)` |
-| bitwise_or | `bitwise_or(expr1, expr2)` or `expr1.bitwise_or(expr2)` |
-| bitwise_xor | `bitwise_xor(expr1, expr2)` or `expr1.bitwise_xor(expr2)` |
-| bitwise_shift_right | `bitwise_shift_right(expr1, expr2)` or `expr1.bitwise_shift_right(expr2)` |
-| bitwise_shift_left | `bitwise_shift_left(expr1, expr2)` or `expr1.bitwise_shift_left(expr2)` |
+:::{note}
+Since `&&` and `||` are existed as logical operators in Rust, but those are not overloadable and not works with expression API.
+:::
## Comparison Expressions
-| Function | Notes |
-| -------- | --------------------- |
-| eq | `expr1.eq(expr2)` |
-| gt | `expr1.gt(expr2)` |
-| gt_eq | `expr1.gt_eq(expr2)` |
-| lt | `expr1.lt(expr2)` |
-| lt_eq | `expr1.lt_eq(expr2)` |
-| not_eq | `expr1.not_eq(expr2)` |
+| Function | Method | Explanation |
Review Comment:
this might be confusing a little bit? if we compare sql and dataframe functions, we may want to name it differently? like `SQL built in function`, `DataFrame function`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1348896380
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
+| Operation | Syntax | Method | Function |
+| ------------------------- | -------- | -------------------------- | --------------------------- |
+| Arithmetic addition | `x + y` | `x.add(y)` | `add(x, y)` |
+| Arithmetic subtraction | `x - y` | `x.sub(y)` | `sub(x, y)` |
+| Arithmetic multiplication | `x * y` | `x.mul(y)` | `mul(x, y)` |
+| Arithmetic division | `x / y` | `x.div(y)` | `div(x, y)` |
+| Arithmetic remainder | `x % y` | `x.r#mod(y)` | `r#mod(x, y)` |
+| Arithmetic negation | `-x` | `x.neg()` | `neg(x, y)` |
+| Bitwise AND | `x & y` | `x.bitwise_and(y)` | `bitwise_and(x, y)` |
+| Bitwise OR | `x \| y` | `x.bitwise_or(y)` | `bitwise_or(x, y)` |
+| Bitwise XOR | `x ^ y` | `x.bitwise_xor(y)` | `bitwise_xor(x, y)` |
+| Bitwise left shift | `x << y` | `x.bitwise_shift_left(y)` | `bitwise_shift_left(x, y)` |
+| Bitwise right shift | `x >> y` | `x.bitwise_shift_right(y)` | `bitwise_shift_right(x, y)` |
+| Logical NOT | `!x` | `x.not()` | `not(x)` |
+
+:::{note}
+`mod` is a reserved keyword in Rust, we use a raw identifier to name the `r#mod` function.
+:::
+
+:::{note}
+`!` is a bitwise or logical complement operator in Rust, but it only works as a logical NOT in expression API.
+:::
## Boolean Expressions
-| Function | Notes |
-| -------- | ----------------------------------------- |
-| and | `and(expr1, expr2)` or `expr1.and(expr2)` |
-| or | `or(expr1, expr2)` or `expr1.or(expr2)` |
-| not | `not(expr)` or `expr.not()` |
-
-## Bitwise expressions
+| Function | Method | Explanation |
+| ----------- | ---------- | ----------- |
+| `and(x, y)` | `x.and(y)` | Logical AND |
+| `or(x, y)` | `x.or(y)` | Logical OR |
+| `not(x)` | `x.not()` | Logical NOT |
-| Function | Notes |
-| ------------------- | ------------------------------------------------------------------------- |
-| bitwise_and | `bitwise_and(expr1, expr2)` or `expr1.bitwise_and(expr2)` |
-| bitwise_or | `bitwise_or(expr1, expr2)` or `expr1.bitwise_or(expr2)` |
-| bitwise_xor | `bitwise_xor(expr1, expr2)` or `expr1.bitwise_xor(expr2)` |
-| bitwise_shift_right | `bitwise_shift_right(expr1, expr2)` or `expr1.bitwise_shift_right(expr2)` |
-| bitwise_shift_left | `bitwise_shift_left(expr1, expr2)` or `expr1.bitwise_shift_left(expr2)` |
+:::{note}
+Since `&&` and `||` are existed as logical operators in Rust, but those are not overloadable and not works with expression API.
+:::
## Comparison Expressions
-| Function | Notes |
-| -------- | --------------------- |
-| eq | `expr1.eq(expr2)` |
-| gt | `expr1.gt(expr2)` |
-| gt_eq | `expr1.gt_eq(expr2)` |
-| lt | `expr1.lt(expr2)` |
-| lt_eq | `expr1.lt_eq(expr2)` |
-| not_eq | `expr1.not_eq(expr2)` |
+| Function | Method | Explanation |
Review Comment:
When I read the doc I cannot understand quickly when should I use method and when should I use function, thats my concern
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346067845
##########
docs/source/conf.py:
##########
@@ -118,4 +118,4 @@
myst_heading_anchors = 3
# enable nice rendering of checkboxes for the task lists
-myst_enable_extensions = [ "tasklist"]
+myst_enable_extensions = ["colon_fence", "deflist", "tasklist"]
Review Comment:
not sure if its related to this PR description 🤔
##########
docs/source/conf.py:
##########
@@ -118,4 +118,4 @@
myst_heading_anchors = 3
# enable nice rendering of checkboxes for the task lists
-myst_enable_extensions = [ "tasklist"]
+myst_enable_extensions = ["colon_fence", "deflist", "tasklist"]
Review Comment:
not sure if its related to this PR description 🤔
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346071477
##########
docs/source/user-guide/expressions.md:
##########
@@ -213,14 +233,14 @@ Unlike to some databases the math functions in Datafusion works the same way as
## Regular Expressions
-| Function | Notes |
+| Function | Explanation |
Review Comment:
Prob Description will be a better word here? 🤔
##########
docs/source/user-guide/expressions.md:
##########
@@ -213,14 +233,14 @@ Unlike to some databases the math functions in Datafusion works the same way as
## Regular Expressions
-| Function | Notes |
+| Function | Explanation |
Review Comment:
Prob Description will be a better word here? 🤔
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide, Add `std::ops` operations to `prelude`, and add `not()` expr_fn [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346731984
##########
docs/source/conf.py:
##########
@@ -118,4 +118,4 @@
myst_heading_anchors = 3
# enable nice rendering of checkboxes for the task lists
-myst_enable_extensions = [ "tasklist"]
+myst_enable_extensions = ["colon_fence", "deflist", "tasklist"]
Review Comment:
I add some [admonitions](https://myst-parser.readthedocs.io/en/latest/syntax/admonitions.html) and [definition list](https://myst-parser.readthedocs.io/en/latest/syntax/optional.html#definition-lists) to the page that require `colon_fence` and `deflist` extensions.
Like this
https://github.com/apache/arrow-datafusion/blob/da033810b192f24a8fb63fb8e5d509e6469b482a/docs/source/user-guide/expressions.md?plain=1#L47-L50
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1751797370
Doc is definitely looks more solid and consistent now, thanks @ongchi
For the new methods introduced for `Expr` trait we may need to cover them by unit tests.
Another thing comes to my mind if we support both syntaxes, we should prob have some notification when new operator is introduced.
It might be a common trait that will require during compilation for another syntax to be implemented
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1353069618
##########
datafusion/expr/src/expr_rewriter/mod.rs:
##########
@@ -265,7 +265,6 @@ mod test {
use arrow::datatypes::DataType;
use datafusion_common::tree_node::{RewriteRecursion, TreeNode, TreeNodeRewriter};
use datafusion_common::{DFField, DFSchema, ScalarValue};
- use std::ops::Add;
Review Comment:
Implementing `std::ops::Add` means you can write something like `expr1 + expr2`.
##########
datafusion/expr/src/expr.rs:
##########
@@ -1032,6 +1033,88 @@ impl Expr {
}
}
+impl Expr {
+ // Operation methods from traits.
+
+ //
+ // Arithmetic operations
+ //
+
+ /// Return `self + other`
+ #[allow(clippy::should_implement_trait)]
Review Comment:
> user may require to include specific traits from std::ops::* in their source.
I think using the `std::ops` in their traits is the less surprising API and is the "idiomatic" rust way to do this. I think we should remove these methods and just use `std::ops`
> Or, should we re-export these traits in prelude?
This would be ok with me. Alternately we could add an example to https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html saying that it is possible to use the `std::ops` and show an example of `a + b` or the equivalent
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1752012139
> Another thing comes to my mind if we support both syntaxes, we should prob have some notification when new operator is introduced. It might be a common trait that will require during compilation for another syntax to be implemented
I think we should reorganize test cases in datafusion-expr and make some improvements by creating a test module. Currently, these tests do not cover all use cases. The tests may follow the order/structure from expression API documentation, which would make it easier to identify which parts needs to be improve.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1345522939
##########
datafusion/expr/src/expr.rs:
##########
@@ -1032,6 +1033,88 @@ impl Expr {
}
}
+impl Expr {
+ // Operation methods from traits.
+
+ //
+ // Arithmetic operations
+ //
+
+ /// Return `self + other`
+ #[allow(clippy::should_implement_trait)]
Review Comment:
Clippy fires warnings because these methods have same name on trait methods in `std::ops`. Without these methods, user may require to include specific traits from `std::ops::*` in their source. Or, should we re-export these traits in prelude?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1746036365
Several functions added in this PR:
- Functions already listed in documentation but not implemented:
- not
- eq
- not_eq
- gt
- gt_eq
- lt
- lt_eq
- Additional arithmetic functions introduced in this PR:
- add
- sub
- mul
- div
- r#mod
- neg
Few more questions:
- These implementations and tests already exist somewhere. Do these still have to add test code here?
- `mod` is a reserved keyword in Rust. The reminder function named here to be able consistant with SQL function naming, but does it ok to `datafusion`?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1348897280
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
Review Comment:
Like in example below the function name comes first and description is last,
In this particular example it looks in reverse order
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1746056637
Folder links does not work in the document. Change all file links to GitHub, which would be easier to browse in this commit 046e020a1b7eab4c84aee6da0716efc92694e16d
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1348893490
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
+| Operation | Syntax | Method | Function |
+| ------------------------- | -------- | -------------------------- | --------------------------- |
+| Arithmetic addition | `x + y` | `x.add(y)` | `add(x, y)` |
+| Arithmetic subtraction | `x - y` | `x.sub(y)` | `sub(x, y)` |
+| Arithmetic multiplication | `x * y` | `x.mul(y)` | `mul(x, y)` |
+| Arithmetic division | `x / y` | `x.div(y)` | `div(x, y)` |
+| Arithmetic remainder | `x % y` | `x.r#mod(y)` | `r#mod(x, y)` |
+| Arithmetic negation | `-x` | `x.neg()` | `neg(x, y)` |
+| Bitwise AND | `x & y` | `x.bitwise_and(y)` | `bitwise_and(x, y)` |
+| Bitwise OR | `x \| y` | `x.bitwise_or(y)` | `bitwise_or(x, y)` |
+| Bitwise XOR | `x ^ y` | `x.bitwise_xor(y)` | `bitwise_xor(x, y)` |
+| Bitwise left shift | `x << y` | `x.bitwise_shift_left(y)` | `bitwise_shift_left(x, y)` |
+| Bitwise right shift | `x >> y` | `x.bitwise_shift_right(y)` | `bitwise_shift_right(x, y)` |
+| Logical NOT | `!x` | `x.not()` | `not(x)` |
+
+:::{note}
+`mod` is a reserved keyword in Rust, we use a raw identifier to name the `r#mod` function.
+:::
+
+:::{note}
+`!` is a bitwise or logical complement operator in Rust, but it only works as a logical NOT in expression API.
+:::
## Boolean Expressions
-| Function | Notes |
-| -------- | ----------------------------------------- |
-| and | `and(expr1, expr2)` or `expr1.and(expr2)` |
-| or | `or(expr1, expr2)` or `expr1.or(expr2)` |
-| not | `not(expr)` or `expr.not()` |
-
-## Bitwise expressions
+| Function | Method | Explanation |
+| ----------- | ---------- | ----------- |
+| `and(x, y)` | `x.and(y)` | Logical AND |
+| `or(x, y)` | `x.or(y)` | Logical OR |
+| `not(x)` | `x.not()` | Logical NOT |
-| Function | Notes |
-| ------------------- | ------------------------------------------------------------------------- |
-| bitwise_and | `bitwise_and(expr1, expr2)` or `expr1.bitwise_and(expr2)` |
-| bitwise_or | `bitwise_or(expr1, expr2)` or `expr1.bitwise_or(expr2)` |
-| bitwise_xor | `bitwise_xor(expr1, expr2)` or `expr1.bitwise_xor(expr2)` |
-| bitwise_shift_right | `bitwise_shift_right(expr1, expr2)` or `expr1.bitwise_shift_right(expr2)` |
-| bitwise_shift_left | `bitwise_shift_left(expr1, expr2)` or `expr1.bitwise_shift_left(expr2)` |
+:::{note}
+Since `&&` and `||` are existed as logical operators in Rust, but those are not overloadable and not works with expression API.
+:::
## Comparison Expressions
-| Function | Notes |
-| -------- | --------------------- |
-| eq | `expr1.eq(expr2)` |
-| gt | `expr1.gt(expr2)` |
-| gt_eq | `expr1.gt_eq(expr2)` |
-| lt | `expr1.lt(expr2)` |
-| lt_eq | `expr1.lt_eq(expr2)` |
-| not_eq | `expr1.not_eq(expr2)` |
+| Function | Method | Explanation |
Review Comment:
Id rather about giving more meaningful name, Function vs Method is confusing imho
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1747137609
Thanks @ongchi for the PR, I left some minor comments
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346747930
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
Review Comment:
What about this?
- Add `Symbol` column
- Move `Function` column comes before `Method`
- Rename `Operation` to `Explanation` or `Description` maybe?
`| Symbol | Syntax | Function | Method | Explanation |`
Does it work for you?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1761905700
Converting to draft as this PR is not waiting on feedback anymore. Please mark it ready to review when ready.
I do realize when writing documentation, often deficiencies in the existing APIs become clearer
One suggestion I had was to split out the changes to the documentation from the changes to how the traits work.
> I think we should reorganize test cases in datafusion-expr and make some improvements by creating a test module. Currently, these tests do not cover all use cases. The tests may follow the order/structure from expression API documentation, which would make it easier to identify which parts needs to be improve.
I agree -- @ongchi could you please file a follow on ticket describing this work? Perhaps it would be a good first step in updating how the traits worked: consolidate all the existing tests we do have, so the scope of the "to do" work becomes clear.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1350501245
##########
datafusion/expr/src/expr.rs:
##########
@@ -1646,4 +1728,89 @@ mod test {
Ok(())
}
+
+ #[test]
+ fn test_arithmetic_ops() {
+ assert_eq!(
+ format!("{}", lit(1u32).add(lit(2u32))),
+ "UInt32(1) + UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).sub(lit(2u32))),
+ "UInt32(1) - UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).mul(lit(2u32))),
+ "UInt32(1) * UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).div(lit(2u32))),
+ "UInt32(1) / UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).mod_(lit(2u32))),
+ "UInt32(1) % UInt32(2)"
+ );
+ assert_eq!(format!("{}", lit(1u32).neg()), "(- UInt32(1))");
+ }
+
+ #[test]
+ fn test_bitwise_ops() {
+ assert_eq!(
+ format!("{}", lit(1u32).bitwise_and(lit(2u32))),
+ "UInt32(1) & UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).bitwise_or(lit(2u32))),
+ "UInt32(1) | UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).bitwise_xor(lit(2u32))),
+ "UInt32(1) BIT_XOR UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).bitwise_shift_left(lit(2u32))),
+ "UInt32(1) << UInt32(2)"
+ );
+ assert_eq!(
+ format!("{}", lit(1u32).bitwise_shift_right(lit(2u32))),
+ "UInt32(1) >> UInt32(2)"
+ );
+ }
+
+ #[test]
+ fn test_logical_ops() {
Review Comment:
awesome. In addition to planner test would be great to have same tests on Dataframe level to prove operators returns correct values.
Update examples and factor out the common interface (to make sure new added operator will be covered by all kind of syntaxes) can be done is follow up PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#issuecomment-1764588276
> Converting to draft as this PR is not waiting on feedback anymore. Please mark it ready to review when ready.
>
> I do realize when writing documentation, often deficiencies in the existing APIs become clearer
>
> One suggestion I had was to split out the changes to the documentation from the changes to how the traits work.
>
> > I think we should reorganize test cases in datafusion-expr and make some improvements by creating a test module. Currently, these tests do not cover all use cases. The tests may follow the order/structure from expression API documentation, which would make it easier to identify which parts needs to be improve.
>
> I agree -- @ongchi could you please file a follow on ticket describing this work? Perhaps it would be a good first step in updating how the traits worked: consolidate all the existing tests we do have, so the scope of the "to do" work becomes clear.
Sure, I'm happy to do that.
I'll focus on correctness of the documentation in this PR with minimal code changes. Any further changes can depend on other users' input.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1350496154
##########
datafusion/expr/src/expr.rs:
##########
@@ -1032,6 +1032,88 @@ impl Expr {
}
}
+impl Expr {
+ // Operation methods from traits.
+
+ //
+ // Arithmetic operations
+ //
+
+ /// Return `self + other`
+ #[allow(clippy::should_implement_trait)]
+ pub fn add(self, other: Expr) -> Expr {
+ std::ops::Add::add(self, other)
+ }
+
+ /// Return `self - other`
+ #[allow(clippy::should_implement_trait)]
+ pub fn sub(self, other: Expr) -> Expr {
+ std::ops::Sub::sub(self, other)
+ }
+
+ /// Return `self * other`
+ #[allow(clippy::should_implement_trait)]
+ pub fn mul(self, other: Expr) -> Expr {
+ std::ops::Mul::mul(self, other)
+ }
+
+ /// Return `self / other`
+ #[allow(clippy::should_implement_trait)]
+ pub fn div(self, other: Expr) -> Expr {
+ std::ops::Div::div(self, other)
+ }
+
+ /// Return `self % other`
+ pub fn mod_(self, other: Expr) -> Expr {
Review Comment:
underscore in naming might be confusing
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346734730
##########
docs/source/user-guide/expressions.md:
##########
@@ -213,14 +233,14 @@ Unlike to some databases the math functions in Datafusion works the same way as
## Regular Expressions
-| Function | Notes |
+| Function | Explanation |
Review Comment:
The column changed form `Notes` to `Explanation` because some note admonitions added to this page. `Description` would also work for me.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346742806
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
+| Operation | Syntax | Method | Function |
+| ------------------------- | -------- | -------------------------- | --------------------------- |
+| Arithmetic addition | `x + y` | `x.add(y)` | `add(x, y)` |
+| Arithmetic subtraction | `x - y` | `x.sub(y)` | `sub(x, y)` |
+| Arithmetic multiplication | `x * y` | `x.mul(y)` | `mul(x, y)` |
+| Arithmetic division | `x / y` | `x.div(y)` | `div(x, y)` |
+| Arithmetic remainder | `x % y` | `x.r#mod(y)` | `r#mod(x, y)` |
+| Arithmetic negation | `-x` | `x.neg()` | `neg(x, y)` |
+| Bitwise AND | `x & y` | `x.bitwise_and(y)` | `bitwise_and(x, y)` |
+| Bitwise OR | `x \| y` | `x.bitwise_or(y)` | `bitwise_or(x, y)` |
+| Bitwise XOR | `x ^ y` | `x.bitwise_xor(y)` | `bitwise_xor(x, y)` |
+| Bitwise left shift | `x << y` | `x.bitwise_shift_left(y)` | `bitwise_shift_left(x, y)` |
+| Bitwise right shift | `x >> y` | `x.bitwise_shift_right(y)` | `bitwise_shift_right(x, y)` |
+| Logical NOT | `!x` | `x.not()` | `not(x)` |
+
+:::{note}
+`mod` is a reserved keyword in Rust, we use a raw identifier to name the `r#mod` function.
+:::
+
+:::{note}
+`!` is a bitwise or logical complement operator in Rust, but it only works as a logical NOT in expression API.
+:::
## Boolean Expressions
-| Function | Notes |
-| -------- | ----------------------------------------- |
-| and | `and(expr1, expr2)` or `expr1.and(expr2)` |
-| or | `or(expr1, expr2)` or `expr1.or(expr2)` |
-| not | `not(expr)` or `expr.not()` |
-
-## Bitwise expressions
+| Function | Method | Explanation |
+| ----------- | ---------- | ----------- |
+| `and(x, y)` | `x.and(y)` | Logical AND |
+| `or(x, y)` | `x.or(y)` | Logical OR |
+| `not(x)` | `x.not()` | Logical NOT |
-| Function | Notes |
-| ------------------- | ------------------------------------------------------------------------- |
-| bitwise_and | `bitwise_and(expr1, expr2)` or `expr1.bitwise_and(expr2)` |
-| bitwise_or | `bitwise_or(expr1, expr2)` or `expr1.bitwise_or(expr2)` |
-| bitwise_xor | `bitwise_xor(expr1, expr2)` or `expr1.bitwise_xor(expr2)` |
-| bitwise_shift_right | `bitwise_shift_right(expr1, expr2)` or `expr1.bitwise_shift_right(expr2)` |
-| bitwise_shift_left | `bitwise_shift_left(expr1, expr2)` or `expr1.bitwise_shift_left(expr2)` |
+:::{note}
+Since `&&` and `||` are existed as logical operators in Rust, but those are not overloadable and not works with expression API.
+:::
## Comparison Expressions
-| Function | Notes |
-| -------- | --------------------- |
-| eq | `expr1.eq(expr2)` |
-| gt | `expr1.gt(expr2)` |
-| gt_eq | `expr1.gt_eq(expr2)` |
-| lt | `expr1.lt(expr2)` |
-| lt_eq | `expr1.lt_eq(expr2)` |
-| not_eq | `expr1.not_eq(expr2)` |
+| Function | Method | Explanation |
Review Comment:
I think this might be OK because it is an `Expression API` page.
The function name between SQL built-in functions and expression functions could keep the same name for the same purpose. If not, maybe we should create a function comparison page to list differences among them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346079036
##########
docs/source/user-guide/expressions.md:
##########
@@ -22,60 +22,89 @@
DataFrame methods such as `select` and `filter` accept one or more logical expressions and there are many functions
available for creating logical expressions. These are documented below.
-Expressions can be chained together using a fluent-style API:
+:::{tip}
+Most functions and methods may receive and return an `Expr`, which can be chained together using a fluent-style API:
```rust
// create the expression `(a > 6) AND (b < 7)`
col("a").gt(lit(6)).and(col("b").lt(lit(7)))
```
+:::
+
## Identifiers
-| Function | Notes |
-| -------- | -------------------------------------------- |
-| col | Reference a column in a dataframe `col("a")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------- |
+| col(ident) | Reference a column in a dataframe `col("a")` |
## Literal Values
-| Function | Notes |
-| -------- | -------------------------------------------------- |
-| lit | Literal value such as `lit(123)` or `lit("hello")` |
+| Function | Explanation |
+| ---------- | -------------------------------------------------- |
+| lit(value) | Literal value such as `lit(123)` or `lit("hello")` |
+
+:::{note}
+value
+: A type which implement `Literal`
+:::
+
+## Basic Operators
+
Review Comment:
I think we need to align columns.
operation sounds like a function description, which called Explanataion below and comes as the last column. Would be nice to stick to consistent table format
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346065716
##########
datafusion/expr/src/expr_fn.rs:
##########
@@ -103,6 +105,50 @@ pub fn or(left: Expr, right: Expr) -> Expr {
))
}
+// There is a solid implementation elsewhere. We are going to wrap it up as a function.
+macro_rules! naive_unary_fn_impl {
+ ($func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(expr: Expr) -> Expr {
+ expr.$func()
+ }
+ };
+}
+
+macro_rules! naive_binary_fn_impl {
+ ($func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(left: Expr, right: Expr) -> Expr {
+ left.$func(right)
+ }
+ };
+ ($func:ident, $from_func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(left: Expr, right: Expr) -> Expr {
+ left.$from_func(right)
+ }
+ };
+}
+
+// Arithmetic expression
+naive_binary_fn_impl!(add, "Arithmetic addition");
+naive_binary_fn_impl!(sub, "Arithmetic subtraction");
+naive_binary_fn_impl!(mul, "Arithmetic multiplication");
+naive_binary_fn_impl!(div, "Arithmetic division");
+naive_binary_fn_impl!(r#mod, r#mod, "Arithmetic remainder");
Review Comment:
is it a typo? shouldn't be it
```
naive_binary_fn_impl!(r#mod, rem, "Arithmetic remainder");
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add operator section to user guide [arrow-datafusion]
Posted by "ongchi (via GitHub)" <gi...@apache.org>.
ongchi commented on code in PR #7732:
URL: https://github.com/apache/arrow-datafusion/pull/7732#discussion_r1346722075
##########
datafusion/expr/src/expr_fn.rs:
##########
@@ -103,6 +105,50 @@ pub fn or(left: Expr, right: Expr) -> Expr {
))
}
+// There is a solid implementation elsewhere. We are going to wrap it up as a function.
+macro_rules! naive_unary_fn_impl {
+ ($func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(expr: Expr) -> Expr {
+ expr.$func()
+ }
+ };
+}
+
+macro_rules! naive_binary_fn_impl {
+ ($func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(left: Expr, right: Expr) -> Expr {
+ left.$func(right)
+ }
+ };
+ ($func:ident, $from_func:ident, $doc:expr) => {
+ #[doc = $doc]
+ pub fn $func(left: Expr, right: Expr) -> Expr {
+ left.$from_func(right)
+ }
+ };
+}
+
+// Arithmetic expression
+naive_binary_fn_impl!(add, "Arithmetic addition");
+naive_binary_fn_impl!(sub, "Arithmetic subtraction");
+naive_binary_fn_impl!(mul, "Arithmetic multiplication");
+naive_binary_fn_impl!(div, "Arithmetic division");
+naive_binary_fn_impl!(r#mod, r#mod, "Arithmetic remainder");
Review Comment:
Yes, you're right.
I'm intended to do that at first, but since `Expr::r#mod` also implemented in this PR, this can be
```
naive_binary_fn_impl!(r#mod, "Arithmetic remainder");
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org