Skip to content

Conversation

ovr
Copy link
Contributor

@ovr ovr commented Dec 22, 2020

Hello!

Thanks

@coveralls
Copy link

coveralls commented Dec 22, 2020

Pull Request Test Coverage Report for Build 443038537

  • 86 of 91 (94.51%) changed or added relevant lines in 4 files are covered.
  • 32 unchanged lines in 10 files lost coverage.
  • Overall coverage decreased (-0.6%) to 91.363%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/mod.rs 10 11 90.91%
tests/sqlparser_common.rs 22 23 95.65%
src/parser.rs 6 9 66.67%
Files with Coverage Reduction New Missed Lines %
src/dialect/ansi.rs 1 85.71%
src/test_utils.rs 1 82.81%
src/dialect/mysql.rs 2 80.0%
src/dialect/postgresql.rs 2 75.0%
src/dialect/snowflake.rs 2 75.0%
src/dialect/sqlite.rs 2 80.0%
src/dialect/generic.rs 4 60.0%
src/dialect/mssql.rs 4 66.67%
src/parser.rs 4 88.82%
src/ast/mod.rs 10 77.78%
Totals Coverage Status
Change from base Build 303848626: -0.6%
Covered Lines: 4845
Relevant Lines: 5303

💛 - Coveralls

@ovr ovr force-pushed the support-explain-analyze branch from f236c37 to 05eda1a Compare December 22, 2020 16:10
@ovr
Copy link
Contributor Author

ovr commented Dec 22, 2020

Hello @Dandandan @nickolay

Can you please take a look?

Thanks!

@Dandandan
Copy link
Contributor

Thanks for your PR @ovr ! Looking good. I left some review comments

@ovr
Copy link
Contributor Author

ovr commented Dec 24, 2020

@Dandandan, Thank for your comments, I've updated PR. Added ExplainLevel enum to support VERBOSE/ANALYZE levels.

@ovr ovr force-pushed the support-explain-analyze branch 5 times, most recently from 62592d5 to 8e0e691 Compare December 24, 2020 11:35
src/ast/mod.rs Outdated
pub enum Statement {
// EXPLAIN
Explain {
level: Option<ExplainLevel>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's not an enum but rather a combination of options.
see: https://www.postgresql.org/docs/9.1/sql-explain.html

I think all the following are correct:

EXPLAIN
EXPLAIN ANALYZE
EXPLAIN VERBOSE
EXPLAIN ANALYZE VERBOSE

So maybe this would be a better encoding?

Explain {
    analyze: bool
    verbose: bool
}

Copy link
Contributor Author

@ovr ovr Dec 24, 2020

Choose a reason for hiding this comment

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

Yeap, you are right.

I checked What combintations work with PostgreSQL:

EXPLAIN VERBOSE SELECT * from test;
EXPLAIN ANALYSE SELECT * from test;
EXPLAIN ANALYZE VERBOSE SELECT * from test;

What doesnt work:

EXPLAIN VERBOSE ANALYZE SELECT * from test;

Find very funny thing, that they support ANALYSE, but it's not mentioned inside docs.

I've updated my PR.

@ovr ovr force-pushed the support-explain-analyze branch from 8e0e691 to 5b2658c Compare December 24, 2020 21:33
@ovr ovr force-pushed the support-explain-analyze branch from 5b2658c to f0801db Compare December 24, 2020 21:42
@Dandandan Dandandan merged commit 17f2930 into apache:main Dec 28, 2020
@Dandandan
Copy link
Contributor

Thanks a lot @ovr for your contribution!

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.

3 participants