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

feat/adding scalar-variable expression #1260

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

devanbenz
Copy link
Contributor

@coveralls
Copy link

coveralls commented May 5, 2024

Pull Request Test Coverage Report for Build 8973785216

Details

  • 9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 89.196%

Totals Coverage Status
Change from base Build 8973304753: 0.003%
Covered Lines: 24314
Relevant Lines: 27259

💛 - Coveralls

@@ -793,6 +793,11 @@ pub enum Expr {
OuterJoin(Box<Expr>),
/// A reference to the prior level in a CONNECT BY clause.
Prior(Box<Expr>),
/// Scalar variable creation e.g. `[@]foo INT`
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a link to some documentation that uses this expression? I think that would be helpful in this case - e.g I'm unfamiliar with the syntax and unable to tell how it's used as a result

@@ -793,6 +793,11 @@ pub enum Expr {
OuterJoin(Box<Expr>),
/// A reference to the prior level in a CONNECT BY clause.
Prior(Box<Expr>),
/// Scalar variable creation e.g. `[@]foo INT`
ScalarVariable {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, it looks like we're missing implementation in the PR for how the parser actually parses/constructs the ScalarVariable expression or is that intended?

@devanbenz
Copy link
Contributor Author

@iffyio it is likely that this PR will not be needed, there is another route I am taking with the Scalar Variable in datafusion 👍 will leave it up just in case though.

@alamb alamb marked this pull request as draft May 13, 2024 19:32
@alamb
Copy link
Contributor

alamb commented May 13, 2024

marking as draft as we sort out what to do here

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