MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:V Narayanan Date:November 18 2008 6:04am
Subject:Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)
Bug#39616
View as plain text  
Hi Ramil,

Thanks a ton for the detailed comments.

In the new patch that I have committed, I have addressed all the issues 
pointed out
by you.

For the tests I did not use a stored data file but instead used 
--write_file.

I have run

perl ~/internals/dev/dgcov/dgcov.pl --uncommitted

and found that the new lines I have added have been tested.

Narayanan

Ramil Kalimullin wrote:
> Hi!
>
> on 11.11.2008 16:53 V Narayanan wrote:
>> #At file:///home/narayanan/Work/mysql/W-M/mysql-5.0-bugteam-39616/
>>
>>  2712 V Narayanan    2008-11-11
>>       Bug#39616: Missing quotes from .CSV crashes server
>
> The patch looks good, see minor suggestions below.
> Ok to push with a test case (you can either use a stored data
> file or create one in the test).
>
>>       When a CSV file contained comma separated elements       that 
>> were not enclosed in quotes, it was causing the
>>       mysql server to crash.
>>             The algorithm that parsed the content of a row in
>>       mysql 5.0 was assuming that the values of the fields
>>       in a .CSV file will be enclosed in quotes and will be
>>       separated by commas.
>>             This was causing the algorithm to fail when the content
>>       of the file resembled the following
>>       3,"with quotes"
>>       The CSV engine that is part of mysql 5.0 was expecting
>>       the above to be
>>       "3","with quotes"
>>             The above is just one example of where the engine was
>>       failing for what would be recognized as a valid .CSV       file 
>> content otherwise.
>>             The proposed fix changes the previous algorithm being used
>>       to parse rows from the .CSV file to handle two separate
>>       cases
>>             1) When the current field of the row is enclosed in quotes
>>       2) When the current field of the row is not enclosed in quotes
>
> Please add a note (for the Doc team) about '/X' chars handling.
>
>> --- a/sql/examples/ha_tina.cc    2008-03-29 15:50:46 +0000
>> +++ b/sql/examples/ha_tina.cc    2008-11-11 12:53:48 +0000
>> @@ -419,34 +419,68 @@ int ha_tina::find_current_row(byte *buf)
>>    for (Field **field=table->field ; *field ; field++)
>>    {
>>      buffer.length(0);
>> -    mapped_ptr++; // Increment past the first quote
>> -    for(;mapped_ptr != end_ptr; mapped_ptr++)
>> +    /* Handle the case where the first character begins with a quote */
>> +    if (*mapped_ptr == '"')
>>      {
>> -      //Need to convert line feeds!
>> -      if (*mapped_ptr == '"' && -          (((mapped_ptr[1] == ',') 
>> && (mapped_ptr[2] == '"')) || (mapped_ptr == end_ptr -1 )))
>> +      /* Increment past the first quote */
>> +      mapped_ptr++;
>> +      /* Loop through the row to extract the values for the current 
>> field */
>> +      for(;mapped_ptr != end_ptr; mapped_ptr++)
> please, add a space after the first ';'
>
>>        {
>> -        mapped_ptr += 2; // Move past the , and the "
>> -        break;
>> -      } -      if (*mapped_ptr == '\\' && mapped_ptr != (end_ptr - 
>> 1)) -      {
>> -        mapped_ptr++;
>> -        if (*mapped_ptr == 'r')
>> -          buffer.append('\r');
>> -        else if (*mapped_ptr == 'n' )
>> -          buffer.append('\n');
>> -        else if ((*mapped_ptr == '\\') || (*mapped_ptr == '"'))
>> -          buffer.append(*mapped_ptr);
>> -        else  /* This could only happed with an externally created 
>> file */
>> +        /* check for end of the current field */
>> +        if (*mapped_ptr == '"' && +            (mapped_ptr[1] == ',' 
>> || mapped_ptr == end_ptr -1 ))
>> +        {
>> +          /* Move past the , and the " */
>> +          mapped_ptr += 2;
>> +          break;
>> +        } +        if (*mapped_ptr == '\\' && mapped_ptr != (end_ptr 
>> - 1))          {
>> -          buffer.append('\\');
>> +          mapped_ptr++;
>> +          if (*mapped_ptr == 'r')
>> +            buffer.append('\r');
>> +          else if (*mapped_ptr == 'n' )
>> +            buffer.append('\n');
>> +          else if ((*mapped_ptr == '\\') || (*mapped_ptr == '"'))
>> +            buffer.append(*mapped_ptr);
>> +          else  /* This could only happed with an externally created 
>> file */
>> +          {
>> +            buffer.append('\\');
>> +            buffer.append(*mapped_ptr);
>> +          }
>> +        } +        else
>> +        {
>> +          /* +             If end of row occurs here, it means there 
>> has been an error
>> +             in parsing
>> +           */
> Remove an extra space before "*/".
> Please add a note that no last qoute was found - it's the real problem 
> here.
>
>> +          if (mapped_ptr == end_ptr -1) 
>> DBUG_RETURN(HA_ERR_END_OF_FILE);
> DBUG_RETURN(HA_ERR_END_OF_FILE); should be on a separate line.
>
>> +          /* Store current character in the buffer for the field */
>>            buffer.append(*mapped_ptr);
>>          }
>> -      } -      else
>> +      }
>> +    }
>> +    else
>> +    {
>> +      /* Handle the case where the current row does not start with 
>> quotes */
>> +        +      /* Loop through the row to extract the values for the 
>> current field */
>> +      for (;mapped_ptr != end_ptr; mapped_ptr++)
> add a space after ';'
>
> Thank you,
> Ramil.
>

Thread
bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712) Bug#39616V Narayanan11 Nov
  • Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)Bug#39616Ramil Kalimullin12 Nov
    • Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)Bug#39616V Narayanan18 Nov
  • Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)Bug#39616Ingo Strüwing12 Nov
    • Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)Bug#39616V Narayanan18 Nov
      • Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)Bug#39616V Narayanan18 Nov