Skip to content

Dictionaries ddl parser#7209

Merged
alesapin merged 12 commits intomasterfrom
dictionaries_ddl_parser
Oct 9, 2019
Merged

Dictionaries ddl parser#7209
alesapin merged 12 commits intomasterfrom
dictionaries_ddl_parser

Conversation

@alesapin
Copy link
Copy Markdown
Member

@alesapin alesapin commented Oct 7, 2019

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

For changelog. Remove if this is non-significant change.

Category (leave one):

  • Other

Short description (up to few sentences):
Parser for dictionaries DDL without any semantic.

@@ -0,0 +1,164 @@
#include <Parsers/ASTDictionary.h>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought that we can parse free-form syntax, convert to DOM and re-use existing code.

Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov Oct 8, 2019

Choose a reason for hiding this comment

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

It can be better, because source can has custom properties depending on the source itself.

Copy link
Copy Markdown
Member Author

@alesapin alesapin Oct 8, 2019

Choose a reason for hiding this comment

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

Yes, I also plan to convert AST to DOM (Poco::AbstractConfiguration) and reuse the current code in the ExternalLoader. But I don't want to make parser absolutely free-form because dictionaries have concrete structure. SOURCE is the only section where we have absolutely custom properties and it's already represented as function with key-value arguments which can hold any amount of custom properties (also nested).

Or you mean some general conversion function from AST to DOM? I don't think that it's necessary, because all sections except source can be converted straightforward.

@alesapin alesapin marked this pull request as ready for review October 8, 2019 13:30
@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Oct 9, 2019

Docs check failed on master

@alesapin alesapin merged commit ef0b2f5 into master Oct 9, 2019
@BayoNet
Copy link
Copy Markdown
Contributor

BayoNet commented Oct 24, 2019

Do we need any docs here?

@akuzm akuzm added the pr-feature Pull request with new product feature label Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature Pull request with new product feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants