Re: [PATCH] (super)user-loadable mysqld parsers [API stuff]

[Arnold Daniels]

Hi,

This is about all I to say on this subject, otherwise I'm just repeating 
myself.

Arnold


Eric Prud'hommeaux wrote:
> * Arnold Daniels <info@stripped> [2007-09-09 13:10-0400]
>   
>> Eric Prud'hommeaux wrote:
>>     
>>> * Arnold Daniels <info@stripped> [2007-09-08 18:11-0400]
>>>   
>>>       
>>>> Hi Eric,
>>>>     
>>>>         
>>> == API stuff ==
>>>   
>>>       
>>>> I'm not fully agreeing with you here.
>>>>     
>>>>         
>>> You can't strongly disagree with me 'cause i don't have a strong
>>> opinion. I have a vague preference for the approach I've taken, and
>>> will defend it 'till I'm dead^h^hbored. Seriously, I appreciate
>>> this discussion.
>>>   
>>>       
>> Aren't you even considering, I might just convince you ;).
>>     
>
> Oh yes, was flippantly attemptying to imply that.
>
> I'm not super invested in one approach over the other. I implemented
> the prefix-switched parser first and did the protocol-switched one
> after talking to a few people, any of whom are welcome to chime in and
> either back their preference or indicate that they've been convinced
> by your arguments. The prefixed-switch parser was certainly easier for
> me to implement.
>
>   
>>>   
>>>       
>>>>                                       I think changing the API just has 
>>>> to big of an impact on all the different clients. Let's say you're use 
>>>> ODBC or PDO in PHP. There's no way to implement that neatly. I really 
>>>> don't like automatic selection either cause it's to easy to mess up.
>>>>     
>>>>         
>>> My uninformed guess is that PHP, DBI, et al are using mysql_query. The
>>> deployment challenge in that case is to add a call for the new API
>>> entry, mysql_send_query, which is like mysql_query but takes an extra
>>> parameter. Likewise, ODBC's executeQuery function would need to link
>>> to mysql_send_query and have some settable var to indicate the
>>> language code to use.
>>>   
>>>       
>> The point here was not about the examples. But there are a few mysql 
>> clients out there: ODBC, /J, native PHP, etc. On top of those there are 
>> hundreds of other drivers, usually multiple for each programming language. 
>> All of these would need to be changed to support parser switching. Now on 
>> top of those there are thousand and thousands of DB abstraction classes and 
>> libs. All of those would need to be changed as well.
>> Besides that, the API off all of these clients can't break compatibility. 
>> In many cases that means that having a second parameter to choose the 
>> parser is out of the question. Adding a function like you did for the mysql 
>> client isn't a possibility for most clients either, since they are data 
>> access abstraction layers, having to conform to a specific API. Sure in 
>> most cases you can come up with a workaround. But it will be a huge mess.
>>     
>
> The extra parser codes are at the end of the set of protocol codes so
> any client should be able to speak to the extended server (I tested
> with a 5.0 client). Likewise with the things that speak to the client,
> I don't see that they break compatibility when a new API point is
> added. However, their code would need to be visited in order to take
> advantage of the extension points.
>   
They will need to change their API to support this feature. In many 
cases a lot of effort has to be taken to not break BC. Take my DB 
abstraction layer in PHP as example:
  function query($statement, [$value, [$value, [...]]]);  # Values get 
quoted and inserted on ?
  $result = $db->query("SELECT * FROM `mytable` WHERE type=? AND date > 
?", $type, $date);
 
I can't add result type to the query function. Now I don't need to get a 
solution for this, I can find it when needed. I'm not talking about 
impossibilities here, but about high inconvenience.

>   
>>>   
>>>       
>>>> If you don't like to integrate the parser, perhaps a good solution is to
> 
>>>> select it with a local setting, so you could do:
>>>>  SET LOCAL query_parser=PARSER_SPASQL;
>>>>  SELECT ?s WHERE { ?s <foo.bar> "hibbyhop" };
>>>>     
>>>>         
>>> The problem with that is getting back. SET (LOCAL|GLOBAL) is parsed
>>> SQL parser. Each parser would have to implement its own way of getting
>>> back or you'd lose the flexibility of being able to intersperse
>>> queries.
>>>   
>>>       
>> Each parser would need to implement the SET command, I don't see a problem 
>> there. Implementing a specific command `PARSER SPASQL`, could also work. 
>> Though not to add yet another non ANSI keyword, something like `SET PARSER 
>> SPASQL` would be a better alternative. That way you don't need to implement 
>> SET in each parser. Though I think having set in each parser would be a 
>> good idea, since is it the way to control how MySQL acts and you want to be 
>> able to do that no matter what type of query your sending.
>>     
>
> * Arnold Daniels <info@stripped> [2007-09-09 16:54-0400]
>   
>> In any case, Oracle uses a specific keyword, XQuery, to select a different 
>> parser. I really think that is the way to go, regardless of you do or do 
>> not want to embed the XQuery statement.
>>     
>
> Here is a perhaps oversimplifed patch to prefix-switch on the server:
>
> --- sql/sql_parse.cc.orig	2007-08-26 04:36:04.000000000 -0400
> +++ sql/sql_parse.cc	2007-09-05 09:54:19.000000000 -0400
> @@ -5336,7 +5348,24 @@
>  
>      Lex_input_stream lip(thd, inBuf, length);
>  
> -    bool err= parse_sql(thd, &lip, NULL);
> +    bool err;
> +    if (!strncmp(buf, "parser", 6) && buf[6] != 0 && buf[7] == ':')
> {
> +      uchar parser = buf[6]-'0';
> +      if (parser > 0 || parser < PARSERS) {
> +	 buf += 8;
> +	 length -= 8;
> +        struct dynamic_parser_info* p = &dynamic_parsers[parser-1];
> +        if (p->initialize) {
> +	   char* space = (char*)malloc(strlen(lip.get_ptr())+1);
> +	   strcpy(space, lip.get_ptr());
> +
> +	   Dynamic_parser* parser = p->initialize(thd, &space);
> +	   err= parser->parse();
> +	   delete parser;
> +        } else {
> +	   my_error(ER_CANT_FIND_DL_ENTRY ,MYF(0), "initialize"); //!!!not loaded
> +        }
> +      } else {
> +	 my_error(ER_UDF_NO_PATHS, MYF(0)"); // !!! illegal, really
> +	 err= 1;
> +      }
> +    } else {
> +      err= parse_sql(thd, &lip, NULL);
> +    }
> +
>      *found_semicolon= lip.found_semicolon;
>  
>      if (!err)
>   
This is very simplified, but you could indeed implement something here. 
In that cause you leave the bnf code as it is.

Arguing about the exact syntax is as arguing about whether peanut butter 
or cotton candy tastes better, so I won't go there. The main thing is 
that there is no need to change thousands of pieces of code, to 
implement this feature, just one.
> but I'd like to hear from a couple other folks to guage the
> temperature of the list.
>   
Me too.
>
> [ cutting out XQuery stuff for this subthread ]
>