WL#7771: Make sure errors are properly handled in DD API

Affects: Server-8.0   —   Status: Complete

DD API implementation is being developed gradually.
There are many transitional changes, it is not
reasonable to take care of all possible errors for them,
as they will be changed eventually anyway.

This WL is to finalize error handling once the
implementation is more or less stabilized.

All the DD API users needs this. Stabilizing error handling avoids
many hard to debug error scenarios.

User Documentation
==================

No user-visible changes. No user documentation required.
Functional requirement:

* Make sure DD methods do not ignore error reported by Raw_* classes.
  
* Make DD API signature changes to return 'true' if there is some
   unexpected error, and 'false' if operation succeeds.

* Make sure error is propagated up, when there is a error reading/writing
  dd::Properties key-value pair.

* Try to handle kill flag during DD operations. We need Identify scenarios
  when a DD operation should not be allowed if kill flag is set. There are
  some DD operations that might still required to be executed even after
  kill flag is set, we need to take appropriate actions.

* TODO
Write error message to server log.
We should use different error code/message when
we get error from DD API framework, something like,
=> "Data-dictionary error: '%original error message here'".


Non functional requirement:

* Add doxygen comments for functions that return unexpected errors.

* Use DBUG_ENTER() and DBUG_RETURN() for important functions.
The problem is that some of the DD API's and DD API internal methods ignore
the SE errors. This is mainly due to past assumption that where made,
that the operations on DD API would not fail. The action required to fix the
problem is to visit most of DD API methods, study the return code path and
make sure we do not ignore any error raised from SE.
There are following cases to handle,

C1) DD method ignores error reported by Raw_* classes.
The signature of DD methods needs to be changed. E.g:,

Raw_record::find_record                                                        
       
Raw_record::find_last_record                                                   
       
Raw_record::prepare_record_for_update                                          
       
Raw_record::open_record_set                                                    
       
Raw_record_set::next

Dictionary_impl::get_object_by_id
Dictionary_impl::get_object_by_key                                             
       
Dictionary_impl::restore_object_from_record                                    
       
Dictionary_object_table::restore_object_from_record                            
       
  
Transaction::get_object
Transaction::get_charset
Transaction::get_collation                                                     
       
Transaction::get_schema
Transaction::get_tablespace                                                    
       
Transaction::fetch_table_by_se_private_key
Transaction::fetch_schemas
Transaction::get_tables_max_se_private_id                                      
       
  
Schema::get_abstract_table                                                     
       
Schema::get_table
Schema::fetch_table_components                                                 
       
Schema::fetch_tables
Schema::fetch_views                                                            
       

Tables::max_se_private_id                                                      
       

dd_schema_exists
dd_table_exists     


C2) DD method should assert() if something fails.

We should never hit such failures. We mostly fix some other part of source
when we hit these asserts. However we have one case,
E.g., dd::Transaction::open_tables() is not expected to fail and we now
assert if it fails.


C3) DD method should fail (returns 'true') and it requires to call my_error().
E.g: Following are the two new error message added.

* Report error to user if the state of DD object is not valid.
ER_INVALID_DD_OBJECT
  eng "%s dictionary object is invalid. (%.192s)"

* Report error to user if the DD object could not be stored in DD table.
ER_UPDATING_DD_TABLE
  eng "Failed to update %.192s dictionary object."


C4)
DD method invokes SE API. SE API fails and does not report error.
MySQL server needs to invoke my_error()/Handler::print_error(). 
E.g: This is most of
DD classes Raw_* would need to do this, after calling SE API's.

C5)
DD method is forced to check thd->is_error() and take decisions if to
continue or stop. This should never happen.



The high level idea is that a function / operation must always return the
error status. In other words, the DD operations must not return meaningful
data, but only error status.

The error status semantic must be common with the rest of the server. That
is:
  - true means an error happened;
  - false means successful execution.

The meaningful data must be returned as out-parameters.

For example:

  bool get_object_by_id(..., Object *&obj);
  bool is_exists(..., bool &is_exists);

In case operations need to return more than error flag (i.e. error code or
something like that), the return type must not be C/C++ primitive type
(int, long, ...). It must be a dedicated user type (like Error_code).

The following alternatives have been considered:

1. Return meaningful data, store the error status inside some context
(e.g. transaction context).

  Object *get_object_by_id(Transaction *trx, ...);

  if (trx->is_error())
    ...

The main downside of this approach is that it's not clear from the
operation declaration if it returns error or not. Moreover, the operations
prototype can change in the future and there is no way to do compiler-time
check that all errors are handled.

Practically, that means that either the error will not be checked, or there
will be too many [unnecessary] error checking.

2. Return meaningful data, store the error status in a special error
context passed to the operation.

  Object *get_object_by_id(Transaction *trx, ..., Error_ctx &err);

  if (err.is_error())
    ...

If we were to have complex error contexts, that would be fine. Having this
complexity for just an error flag seems to be over the top.

3. C++ exceptions.

The idea was that we could use C++ exceptions within the DD, but provide
exception-free API.