Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Improve user documentation for supported operators and expressions #520

Merged
merged 12 commits into from
Jun 7, 2024

Conversation

andygrove
Copy link
Member

@andygrove andygrove commented Jun 5, 2024

Which issue does this PR close?

N/A

Rationale for this change

The list of supported expressions was incomplete and we did provide any information about known incompatibilities

What changes are included in this PR?

  • Use table instead of list
  • Add more expressions
  • Add notes about know incompatibilities
  • Renamed Negative to UnaryMinus in protobuf for consistency

How are these changes tested?

@andygrove
Copy link
Member Author

I plan on keeping this as draft for a while and will keep updating it as we prepare for the 0.1.0 release.

@andygrove andygrove marked this pull request as ready for review June 7, 2024 16:06
@@ -563,7 +563,7 @@ impl PhysicalPlanner {
let child = self.create_expr(expr.child.as_ref().unwrap(), input_schema)?;
Ok(Arc::new(NotExpr::new(child)))
}
ExprStruct::Negative(expr) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed this because it was inconsistent with Spark naming

@andygrove
Copy link
Member Author

@parthchandra @kazuyukitanimura @huaxingao @viirya This is ready for review now. Thanks.

@andygrove
Copy link
Member Author

@comphead This is the user guide version of the list of supported expressions with notes about known issues. I think this is separate from the list that you are generating for the contributors guide.

| Sort | |
| Hash Aggregate | |
| Limit | |
| Sort-merge Join | Sort-merge join is disabled by default |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, is SMJ disabled by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like I misunderstood this. I have update this.

Copy link
Contributor

@kazuyukitanimura kazuyukitanimura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending ci

| Repeat | Negative argument for number of times to repeat causes exception |
| Replace | |
| Reverse | |
| RLike | Disabled by default. Uses Rust regular expression engine which is not compatible with Java's regexp engine. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: RLike is still under review. Not sure which gets merged first...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this and will add it back in the RLike PR after this one is merged

@andygrove andygrove merged commit f75aeef into apache:main Jun 7, 2024
43 checks passed
@andygrove andygrove deleted the improve-docs branch June 7, 2024 21:15
himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024
…ons (apache#520)

* Improve documentation about supported operators and expressions

* Improve documentation about supported operators and expressions

* more notes

* Add more supported expressions

* rename protobuf Negative to UnaryMinus for consistency

* format

* remove duplicate ASF header

* SMJ not disabled by default

* Update docs/source/user-guide/operators.md

Co-authored-by: Liang-Chi Hsieh <[email protected]>

* Update docs/source/user-guide/operators.md

Co-authored-by: Liang-Chi Hsieh <[email protected]>

* remove RLike

---------

Co-authored-by: Liang-Chi Hsieh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants