Re: bk commit into 5.2 tree (rafal:1.2601) BUG#30938

[Andrei Elkin]

Rafal, hello.

Could we finish with the issue?

>
> Yes, I open a tmp table here so that add_table(t) method below can use
> t.hton() to get a pointer to table's handlerton structure.
>
>>> +    if (!t.open(m_thd))
>>>      {
>>> -      DBUG_PRINT("backup", ("Found table %s for database %s",
>>> -                             t.name().ptr(), t.db().name().ptr()));
>>> +      Table_ref::describe_buf buf;
>>
>> in which case the only possible error is out of memory.
>> However, the new error message is instead. 
>>
>>> +      report_error(ER_BACKUP_TABLE_OPEN,t.describe(buf));
>>
>> Why?
>>
>
> For the following reasons:
>
> 1. Compare these two error messages:
>
> [ERROR] Backup: Out of memory.
>
> and
>
> [ERROR] Backup: Can't open table test.t1.
>
> I think the second one is more informative and better describes the
> problem which was encountered.
>
> 2. When I hit this error, what I know is that open_temporary_table()
> signalled an error. Documentation of this function doesn't specify
> what kind of error it was. Perhaps, in the current tree it can happen
> only because of lack of memory. But who is going to guarantee that it
> will be the case in the future? I strongly object against writing code
> which depends on such implicit assumptions. All assumptions should be
> explicit and written down in the documentation. Otherwise maintaining
> the code becomes a nightmare.
>

I understand that `out of memory' from t.open() might be not that
frequent. Still, we have to distinguish between that error and the
absense of the plugin in your patch.

Have I missed that you really handle that?

cheers,

Andrei