Re: bzr commit into mysql-5.1-bugteam branch (nirbhay.choubey:3554) Bug#54899

[Nirbhay Choubey]

Hi Sergey, Joro,

SET reply_mode = default;  (will be difficult for me to follow the 
style) :-)

On Thursday 18 November 2010 02:30 AM, Sergey Vojtovich wrote:
> Hi Nirbhay, Joro!
>
> SET reply_mode= PETER_GULUTZAN -- :)
>
> On Wed, Nov 17, 2010 at 03:35:51PM +0530, Nirbhay Choubey wrote:
>    
>> Hi Sergey,
>>
>> Please see my comments in-line.
>>
>> On Tuesday 16 November 2010 10:44 PM, Sergey Vojtovich wrote:
>>      
>>> Hi Nirbhay,
>>>
>>> a few use cases that used to work...
>>> Joro: do you think all of these points are valid?
>>>
>>> I run "mysql" with one-database option.
>>> I do not specify database name.
>>> I execute USE test.
>>> I get segmentation fault.
>>> I expect "Database changed".
>>>        
>> This happened as I missed out a check in my last patch,
>> the latest patch deals with this.
>>      
> Joro says: "Heh, that's obviously a valid case."
> We all agree that this case is valid.
> It is to be dealed by the latest patch.
> Issue closed.
>    

Fixed in the last patch.

>    
>>> I run "mysql" with one-database option.
>>> I specify "test" as database name.
>>> I execute USE non-existent-db.
>>> I get "ERROR 1049 (42000): Unknown database 'non-existent-db'".
>>> I expect "Database changed".
>>>        
>> This is expected, as we are running mysql_select_db with the
>> db name given with USE command, as against the older (pre-patch)
>> logic, where (in case of one-database), we used to skip mysql_select_db
>> call.
>>      
> Joro says: "The error is expected imho. you're not allowed to switch
> to non-existent db iirc."
> Nirbhay and Joro agrees.
> Svoj won't object. Yet.
> Probably Svoj will object with different arguments later.
> Issue closed.
>
>    
>>> I run "mysql" with one-database option.
>>> I specify "test" as database name.
>>> I execute CREATE DATABASE test1.
>>> I execute USE test1.
>>> I execute USE test1.
>>> I execute SELECT 1.
>>> I get "1".
>>> I expect "Ignoring query to other database".
>>>        
>> Again, a catch, fixed it in the latest patch.
>>      

Fixed in the last patch. It should show the expected result now.

> Joro says: "Hmm, shouldn't USE fail ? how is the --one-database defined ?
> What is the current database after the above sequence of commands ?"
> Svoj answers in order: "No answer from my side. Could you elaborate?
> Prompt says: 'test1', SELECT DATABASE() says: 'test1'".
> It is to be dealed by the latest patch.
> Joro probably sees a problem with "USE test1" not failing, but that's
> not relevant to this particular case.
> Issue closed.
>    

None of the USE commands above should fail with the
last patch as we are executing each USE commands with
mysql_select_db.(even in the case of one-database) as against
the pre-patch code, where we used to skip the mysql_select_db
in case of one-database.

>    
>>> I run "mysql" with one-database option.
>>> I specify "test" as database name.
>>> I execute USE non-existent-db.
>>> I get "ERROR 1049 (42000): Unknown database 'non-existent-db'".
>>> I execute "\R \d>   ".
>>> I get "test>   ".
>>> I expect "test>   " (actually I would expect 'non-existent-db').
>>>        
>> I expect this, as prompt is created using current_db, and current_db
>> is not supposed to change after 'USE non-existent-db'.
>>      
> Joro says: "no : look on the above example and my answer there : test is fine."
> We all agree that "test>  " is fine, taking into account use case #2.
>
>    

>>> I execute SELECT 1.
>>> I get "Ignoring query to other database".
>>> I expect "1" (actually I would expect 'Ignoring query to other database').
>>>        
>> This is again an expected behavior, as one-database filtering is decided
>> based on the database name provided in the 'USE' command, which is
>> 'non-existent-db' in this scenario.
>>      
> Joro says: "That's an error indeed. it should be "1"."
> Joro and Svoj expect queries to be accepted when one-databse is "test" and
> prompt says "test".
> Nirbhay does not.
> Prior to this patch prompt would be "test>  ", so it is not a regression.
> Issue is not settled.
>    

True, no regressions here. The outcome will be similar for both
pre-patch and post-patch code (the only change is USE non-existent-db
with throw an error message instead of 'Database changes').

1) mysql> \R \d>
     PROMPT set to '\d>'

2) test> USE test;
     Database changed

3) test> SELECT 1;
     +---+
      | 1 |
     +---+
      | 1 |
      +---+

4)  test>USE non-existent-db;
      ERROR 1049 (42000): Unknown database 'non-existent-db'

5)  test> SELECT 1;
      Ignoring query to other database

In the above statement the prompt says 'test', which is correct as the 
last USE
was executed with a non-existing db.

But, IMO, if the user has executed USE to switch to a different database 
(4), means
the *intention* is to execute the next statement(s) (5 and so forth) on 
this new database
and not on 'test'. So, we should start filtering the statements from (5) 
onwards.

And, again IMO,  this is how we should decide to filter, based on the 
USE command,
(be it on an existent or a non-existent database) and not on current_db. 
current_db (i.e
the prompt) is just an indicator that current_db is 'test'.

Regards,
Nirbhay

PS. In the last/latest patch I have modified the help option to align 
more with
the current scenario :

-   "Only update the default database. This is useful for skipping updates "
-   "to other database in the update log.",
+   "Ignore statements except those that occur while the default "
+   "database is the one named at the command line and USE command "
+   "is not executed with some other database.",


> Regards,
> Sergey
>
>    
>> Best,
>> Nirbhay
>>
>>      
>>> Regards,
>>> Sergey
>>>        
>>      
>