Hi, YES!, ripping out parse related code from the parser and using it independently would be superb. Roy and I talked about this a little bit last week and he seems to be interested in doing so too.
> I would actually prefer that the interface for parsing not get wrapped up in > the Session object. This is not be possible right away (explained below), > but I would prefer to see a pointer to a more lightweight and > parser-specific object passed to a parser plugin. For instance, a Node in > an abstract syntax tree. Agreed, I guess there is no need to "work around" the current limitation of the core. We should really redesign this area, since we can :) > So, all of the above functions would need to be altered to not need the > Session object (all of them can be.) and we'd want to pass in the mem_root > pointer to the parse function instead of the Session object. Thanks for pointing out the bits and pieces! I guess I can start looking into this but it might take some time since I haven't quite mastered this area of the codebase yet. After that, I'll try and design a structure and API for the Parse Tree that the parser will work with... though I suspect its not going to be easy. >From your feedback, its now clear in my mind that we need to make various baby-steps to the core before we can seriously consider pushing the parser out... fun. > The basic idea being that the parser should focus on *parsing* and nothing > else. Just parse the query into a tree and pass it back to the caller. > That's it. No need to figure out how to send warnings to a client. No > need for the parser to try and figure out whether the session has access to > an object or not. Just build the parse tree and pass it back. Let other > functions validate the tree. Parsers should parse. That's it. :) ++ > It will be going away shortly, since the frm is almost completely gone. Great news! > I have lots more to talk about, but I'll leave it there for now...just to > get the ideas flowing. Thanks! yeah, would be cool to hear what ideas others have in mind. Cheers, Toru On Mon, Mar 2, 2009 at 1:11 AM, Jay Pipes <[email protected]> wrote: > First of all, before I get to comments inline, I'd like to say...great work > so far! I'm really looking forward to the development of this plugin. :) > The below are more of a stream-of-thought comments, not particularly > well-structured, but hopefully get some thoughts circling. > > Toru Maesaka wrote: >> >> G'day! >> >> As some of you already know, lately I've been experimenting on >> making the current parser pluggable. I've managed to write a demo >> that passes the test suite so I wanted to get some feedbacks on >> what you all think. >> >> The tree is at: >> http://bazaar.launchpad.net/~tmaesaka/drizzle/pluggable-parser >> >> So, what I've done in this tree is simple. I replaced the current entry >> point of the parser (mysql_parse) with a plugin entry point and pushed >> out what was behind mysql_parse() into a mandatory module. >> >> The idea is that a module implements a parser that populates the session >> object using the provided query string, query length and a variable for >> the >> discovered semicolon with this interface: >> >> >> bool (*sql_parse)(Session *session, const char *query, >> const size_t query_len, const char >> **found_semicolon); > > OK, I'll start off with my "big picture" spiel, and then move on to a more > realistic approach... :) > > I would actually prefer that the interface for parsing not get wrapped up in > the Session object. This is not be possible right away (explained below), > but I would prefer to see a pointer to a more lightweight and > parser-specific object passed to a parser plugin. For instance, a Node in > an abstract syntax tree. > > The idea here is that a plugin should not be passed any more information > than it needs to do its job. Clean interfaces ensure that the boundaries of > information are clearly defined and that objects only access those things > about which they "know". > > A parser knows the following: > > * The input type it expects (typically a string of characters) > * A set of tokens (actually this is more the domain of the lexer, but bear > with me...) > * A set of rules -- the grammar of the language > * The production is should output -- the "parse tree" > > What it does *not* need to know about (or care about) is the myriad other > things which are attached to the Session object: > > * Status variables > * Configuration variables > * Diagnostics arenas > * Lock information > * Protocol information for communicating with the client socket > * Security contexts (ACL, RBAC) > * Replicator information > * Transaction information > * Much more, as you know... > > By passing a pointer to the Session object, which is, unfortunately, > designed in a very open, member-public, way, we risk the parser getting > entangled in the ABI for the Session, which is an object bigger than the > parser needs to care about. > > That said, our existing parser unfortunately has the Session object > thingsentangled in itself...which is obvious as soon as you open up the Yacc > file and see things like this all over the place: > > Session *session= YYSession; > session->lex->sql_command= SQLCOM_EMPTY_QUERY; > > The macros at the top of the Yacc file are telling: > > %{ > /* session is passed as an argument to yyparse(), and subsequently > ** to yylex(). > ** The type will be void*, so it must be cast to (Session*) when used. > ** Use the YYSession macro for this. > */ > #define YYPARSE_PARAM yysession > #define YYLEX_PARAM yysession > #define YYSession ((Session *)yysession) > ... > #define Lex (YYSession->lex) > #define Select Lex->current_select > > There are a number of functions that the session object is passed to from > within the parser: > > push_warning_printf() > select_lex->add_table_to_list() > ha_resolve_by_name() > add_field_to_list() > select_lex->alloc_index_hints() > session->add_item_to_list() > negate_expression() > handle_2003_note184_exception() > create_func_cast() > select_lex->nest_last_join() > push_new_name_resolution_context() > select_lex->init_nested_join() > select_lex->end_nested_join() > prepare_schema_table() > > In addition, the session object's mem_root is supplied as an allocator to > most of the Item constructors. > > So, all of the above functions would need to be altered to not need the > Session object (all of them can be.) and we'd want to pass in the mem_root > pointer to the parse function instead of the Session object. > > I'm thinking a goal for the *interface* would be something like the > following for a C plugin: > > bool (*sql_parse) > (ParseTree* node, MEM_ROOT *alloc, const char *query, const size_t > query_len); > > and this for a C++ plugin: > > class SqlParseEvent > { > public: > bool operator()(ParseTree *node, MEM_ROOT *root, std::string query); > } > > The basic idea being that the parser should focus on *parsing* and nothing > else. Just parse the query into a tree and pass it back to the caller. > That's it. No need to figure out how to send warnings to a client. No > need for the parser to try and figure out whether the session has access to > an object or not. Just build the parse tree and pass it back. Let other > functions validate the tree. Parsers should parse. That's it. :) > > After looking hard today at the current parser (which is, BTW, smaller than > MySQL 4.1's parser and less than a third of the size of MySQL 6.0's) I > strongly believe we can reduce the parser by at least another third (not the > grammar, but the productions in the grammar rules) by removing the Session > from the parser and reducing the amount of validation and checking the > parser is doing and pulling that stuff out into separate functions... > >> >> Saying that, things weren't as simple as I wanted it to be and I had to do >> the following to modularize the parser: >> >> - Separate mysql_execute_command() from the parser for a "clean" >> separation of the parser from the core. I've added a wrapper function >> within the core called sql_parse_and_execute() to replace where >> previously parsing and query execution occurred at the same place. >> >> Also, although my original intention was to only introduce one plugin >> function, I had to unfortunately add another one called: >> >> >> sql_parse_vcol_expr(Session *session, const char *vcol_expr, >> const size_t length); >> >> >> this is due to the way unpack_vcol_info_from_frm() in table.cc relies >> on a direct call to the parser to translate the virtual column expression >> from a .frm file into an Item object. So, I needed a way to directly call >> the raw parser without going through housekeeping. >> >> I reaaaaaally don't want this function but to kill this thing, we need to >> come up with a way to create a virtual column object without relying >> on the parser. Either that or I think we can kill this function when >> Drizzle doesn't need .frm files anymore. I think this is currently being >> worked on? > > It will be going away shortly, since the frm is almost completely gone. > >> Anyhow, that's all I have to say for now! It would be great if you guys >> could tell me whether I screwed up miserably or if its worth considering >> :) > > I have lots more to talk about, but I'll leave it there for now...just to > get the ideas flowing. > > Cheers! > > Jay > > _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
