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 parsing ALTER STATISTICS in PostgreSQL #19842

Merged
merged 2 commits into from Aug 4, 2022

Conversation

everly-gif
Copy link
Contributor

Ref #17848 .
Sub-issue of #14015

Changes proposed in this pull request:

  • Proofread ALTER STATISTICS grammar
  • Support parsing ALTER STATISTICS in PostgreSQL
  • Add tests

✅ Builds locally

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2022

Codecov Report

Merging #19842 (b109bbb) into master (84f9266) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master   #19842      +/-   ##
============================================
- Coverage     60.12%   60.11%   -0.01%     
- Complexity     2409     2410       +1     
============================================
  Files          3852     3855       +3     
  Lines         54936    54935       -1     
  Branches       7706     7706              
============================================
- Hits          33031    33026       -5     
- Misses        19070    19075       +5     
+ Partials       2835     2834       -1     
Impacted Files Coverage Δ
...common/statement/ddl/AlterStatisticsStatement.java 0.00% <0.00%> (ø)
...gresql/ddl/PostgreSQLAlterStatisticsStatement.java 0.00% <0.00%> (ø)
...atement/impl/PostgreSQLDDLStatementSQLVisitor.java 85.35% <100.00%> (+0.03%) ⬆️
...l/parser/core/database/visitor/SQLVisitorRule.java 100.00% <100.00%> (ø)
...eterized/jaxb/cases/domain/SQLParserTestCases.java 99.29% <100.00%> (+<0.01%) ⬆️
...tatement/ddl/AlterStatisticsStatementTestCase.java 100.00% <100.00%> (ø)
...handler/distsql/ral/hint/enums/HintSourceType.java 0.00% <0.00%> (-42.86%) ⬇️
.../datasource/pool/creator/DataSourceReflection.java 71.42% <0.00%> (-7.15%) ⬇️
...sistRepositoryConfigurationYamlSwapperFactory.java 100.00% <0.00%> (ø)
...here/infra/yaml/schema/pojo/YamlIndexMetaData.java
... and 15 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@@ -1056,10 +1056,10 @@ foreignServerVersion

alterStatistics
: ALTER STATISTICS
( ifExists? anyName SET STATISTICS signedIconst
Copy link
Member

Choose a reason for hiding this comment

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

Why remove ifExists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't exsits in the PostgreSQL dialect here
https://www.postgresql.org/docs/current/sql-alterstatistics.html

Copy link
Member

Choose a reason for hiding this comment

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

it doesn't exsits in the PostgreSQL dialect here https://www.postgresql.org/docs/current/sql-alterstatistics.html

Can you check this grammar in PostgreSQL source code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean the postgres gram.y file right?

AlterStatsStmt:
			ALTER STATISTICS any_name SET STATISTICS SignedIconst
				{
					AlterStatsStmt *n = makeNode(AlterStatsStmt);

					n->defnames = $3;
					n->missing_ok = false;
					n->stxstattarget = $6;
					$$ = (Node *) n;
				}
			| ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
				{
					AlterStatsStmt *n = makeNode(AlterStatsStmt);

					n->defnames = $5;
					n->missing_ok = true;
					n->stxstattarget = $8;
					$$ = (Node *) n;
				}
			;

There if exists is supported.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, document is sometimes inaccurate, and source code can give us better guidance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for this tip! will keep this in my mind for my next PRs . Ill revert the changes for DDLStatement.g4

Copy link
Member

@strongduanmu strongduanmu left a comment

Choose a reason for hiding this comment

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

@everly-gif Another perfect pr. 👍

@strongduanmu strongduanmu merged commit 3b2cd69 into apache:master Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants