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

Add support for DuckDB's CREATE MACRO statements #897

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

MartinNowak
Copy link
Contributor

  • support scalar and table macros
  • separate from related but different and already complex functions
    parsing to avoid plenty of runtime dialect switches

- support scalar and table macros
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 @MartinNowak -- this is looking pretty close. I left some comments let me know what you think

src/ast/mod.rs Outdated Show resolved Hide resolved
src/ast/mod.rs Outdated Show resolved Hide resolved
src/ast/mod.rs Outdated Show resolved Hide resolved
src/parser.rs Show resolved Hide resolved
tests/sqlparser_duckdb.rs Outdated Show resolved Hide resolved
@MartinNowak MartinNowak force-pushed the duckdb-macros branch 3 times, most recently from 635fd33 to 618ad71 Compare June 19, 2023 08:51
@MartinNowak
Copy link
Contributor Author

Thank you @MartinNowak -- this is looking pretty close. I left some comments let me know what you think

Thanks for the detailed review @alamb, resolved all points except this discussion.

@alamb
Copy link
Contributor

alamb commented Jun 20, 2023

Thanks @MartinNowak -- I am a behind on reviews. I plan to review this tomorrow. Sorry for the delay

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5309894222

  • 65 of 83 (78.31%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.04%) to 86.9%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/tokenizer.rs 1 2 50.0%
src/parser.rs 23 31 74.19%
src/ast/mod.rs 21 30 70.0%
Files with Coverage Reduction New Missed Lines %
src/ast/mod.rs 1 78.34%
Totals Coverage Status
Change from base Build 5279238565: -0.04%
Covered Lines: 14939
Relevant Lines: 17191

💛 - 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.

Looks great to me -- thank you @MartinNowak

@alamb alamb merged commit 75f18ec into apache:main Jun 21, 2023
@MartinNowak MartinNowak deleted the duckdb-macros branch June 28, 2023 08:30
serprex pushed a commit to serprex/sqlparser-rs that referenced this pull request Nov 6, 2023
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