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

Support use of BY NAME quantifier across all set ops #1309

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

alexander-beedie
Copy link
Contributor

@alexander-beedie alexander-beedie commented Jun 12, 2024

It turns out that that use of the "BY NAME" quantifier was being artificially constrained to UNION set ops - however, there are dialects that allow it for all set ops (which actually makes a lot of sense).

I'm about to implement this additional "BY NAME" set op support for the Polars SQL interface (which will need this patch), but the expanded usage is already found in the wild; see Microsoft's U-SQL dialect (as used in Azure Data Lake Analytics) for an example:

Also:

  • Spotted a missing backtick in the docs - added.
  • Fixed some minor typos in comments/docstrings.
  • Added a note to the README that polars also uses the parser 👍

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9478220409

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9481344190

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9481781619

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

@alexander-beedie alexander-beedie force-pushed the set-ops-by-name-quantifier branch from 72b35f2 to 7ca607f Compare June 12, 2024 12:47
@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9482850329

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9482845468

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

Copy link
Contributor

@jmhain jmhain left a comment

Choose a reason for hiding this comment

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

lgtm, many thanks for the typo cleanups! cc @alamb

src/parser/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: Joey Hain <[email protected]>
@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9487455051

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 88.878%

Files with Coverage Reduction New Missed Lines %
src/parser/mod.rs 1 93.09%
Totals Coverage Status
Change from base Build 9439708625: 0.0%
Covered Lines: 26259
Relevant Lines: 29545

💛 - Coveralls

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @jmhain and @alexander-beedie

@@ -210,6 +209,7 @@ licensed as above, without any additional terms or conditions.
[Ballista]: https://github.com/apache/arrow-ballista
[GlueSQL]: https://github.com/gluesql/gluesql
[Opteryx]: https://github.com/mabel-dev/opteryx
[Polars]: https://pola.rs/
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@alamb alamb merged commit 0330f9d into apache:main Jun 17, 2024
10 checks passed
@alexander-beedie alexander-beedie deleted the set-ops-by-name-quantifier branch June 17, 2024 18:48
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