Oops, I meant "from the session object", in my first sentence :) On Mon, Mar 2, 2009 at 11:25 AM, Toru Maesaka <[email protected]> wrote: > 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 >> >> >
-- Toru Maesaka <[email protected]> _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
