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

Deprecated some methods #476

Merged
merged 4 commits into from
Nov 14, 2022

Conversation

ikrivosheev
Copy link
Member

@ikrivosheev ikrivosheev commented Oct 15, 2022

Changes

  • Expr::value accept Into<SimpleExpr> instead Into<Value>
  • Expr::gt accept Into<SimpleExpr> instead Into<Value>
  • Expr::gte accept Into<SimpleExpr> instead Into<Value>
  • Expr::lt accept Into<SimpleExpr> instead Into<Value>
  • Expr::lte accept Into<SimpleExpr> instead Into<Value>
  • Expr::add accept Into<SimpleExpr> instead Into<Value>
  • Expr::div accept Into<SimpleExpr> instead Into<Value>
  • Expr::sub accept Into<SimpleExpr> instead Into<Value>
  • Expr::modulo accept Into<SimpleExpr> instead Into<Value>
  • Expr::left_shift accept Into<SimpleExpr> instead Into<Value>
  • Expr::right_shift accept Into<SimpleExpr> instead Into<Value>
  • Expr::between accept Into<SimpleExpr> instead Into<Value>
  • Expr::not_between accept Into<SimpleExpr> instead Into<Value>
  • Expr::is accept Into<SimpleExpr> instead Into<Value>
  • Expr::is_not accept Into<SimpleExpr> instead Into<Value>
  • Expr::if_null accept Into<SimpleExpr> instead Into<Value>
  • Expr::is_in, Expr::is_not_in accept Into<SimpleExpr> instead Into<Value> and convert it to SimpleExpr::Tuple instead SimpleExpr::Values

Deprecated

  • Expr::greater_than
  • Expr::greater_or_equal
  • Expr::less_than
  • Expr::less_or_equal

@ikrivosheev ikrivosheev self-assigned this Oct 15, 2022
@Razican
Copy link

Razican commented Oct 21, 2022

As talked in Discord, it seems that the Expr::is_in() function makes it tricky to use custom enums, for example, to add a list of values. @billy1624 recommended using this:

Expr::col(Glyph::Id).binary(BinOper::In, Expr::tuple([Expr::cust("1"), Expr::cust("2")]))

But this is a bit cumbersome, so he proposed to simplify that within this PR.

@ikrivosheev
Copy link
Member Author

As talked in Discord, it seems that the Expr::is_in() function makes it tricky to use custom enums, for example, to add a list of values. @billy1624 recommended using this:

Expr::col(Glyph::Id).binary(BinOper::In, Expr::tuple([Expr::cust("1"), Expr::cust("2")]))

But this is a bit cumbersome, so he proposed to simplify that within this PR.

@Razican hello! Thank you, done

@ikrivosheev
Copy link
Member Author

@billy1624 @tyt2y3 can you review the PR?

@ikrivosheev
Copy link
Member Author

@billy1624 @tyt2y3 ping

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

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

Looking great

@ikrivosheev ikrivosheev merged commit fad9224 into SeaQL:master Nov 14, 2022
@ikrivosheev ikrivosheev deleted the featue/deprecated_methods branch November 14, 2022 08:35
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.

None yet

3 participants