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
