This patch fixes:
- MDEV-19284 INSTANT ALTER with ucs2-to-utf16 conversion produces bad data
- MDEV-19285 INSTANT ALTER from ascii_general_ci to latin1_general_ci produces corrupt data
These regressions were introduced in 10.4.3 by:
- MDEV-15564 Avoid table rebuild in ALTER TABLE on collation or charset changes
Changes:
1. Cleanup: Adding a helper method
Field_longstr::csinfo_change_allows_instant_alter(),
to remove some duplicate code in field.cc.
2. Cleanup: removing Type_handler::Charsets_are_compatible() and static
function charsets_are_compatible() and
introducing new methods in the recently added class Charset instead:
- encoding_allows_reinterpret_as()
- encoding_and_order_allow_reinterpret_as()
3. Bug fix: Removing the code that allowed instant conversion for
ascii-to->8bit and ucs2-to->utf16.
This actually fixes MDEV-19284 and MDEV-19285.
4. Bug fix: Adding a helper method Charset::collation_specific_name().
The old corresponding code in Type_handler::Charsets_are_compatible()
was not safe against (badly named) user-defined collations whose
character set name can be longer than collation name.
This also fixes:
MDEV-17299 Assertion `maybe_null' failed in make_sortkey
Note, during merge of the 10.1 version of MDEV-17299,
please use the 10.3 version of the code (i.e. null merge the 10.1 version).
sql_field->key_length was 0 for blob fields when a field was
being added, but Field_blob::character_octet_length() on
subsequent ALTER TABLE's (when the Field object in the old table
already existed). This means mysql_prepare_create_table() couldn't
reliably detect if the keyseg was a prefix.
1. Renaming Type_handler_json to Type_handler_json_longtext
There will be other JSON handlers soon, e.g. Type_handler_json_varchar.
2. Making the code more symmetric for data types:
- Adding a new virtual method
Type_handler::Column_definition_validate_check_constraint()
- Moving JSON-specific code from sql_yacc.yy to
Type_handler_json_longtext::Column_definition_validate_check_constraint()
3. Adding new files sql_type_json.cc and sql_type_json.h
and moving Type_handler+JSON related code into these files.
Allow ALGORITHM=INSTANT (or avoid touching any data)
when changing the collation, or in some cases, the character set,
of a non-indexed CHAR or VARCHAR column. There is no penalty
for subsequent DDL or DML operations, and compatibility with
older MariaDB versions will be unaffected.
Character sets may be changed when the old encoding is compatible
with the new one. For example, changing from ASCII to anything
ASCII-based, or from 3-byte to 4-byte UTF-8 can sometimes be
performed instantly.
This is joint work with Eugene Kosov.
The test cases as well as ALTER_CONVERT_TO, charsets_are_compatible(),
Type_handler::Charsets_are_compatible() are his work.
The Field_str::is_equal(), Field_varstring::is_equal() and
the InnoDB changes were mostly rewritten by me due to conflicts
with MDEV-15563.
Limitations:
Changes of indexed columns will still require
ALGORITHM=COPY. We should allow ALGORITHM=NOCOPY and allow
the indexes to be rebuilt inside the storage engine,
without copying the entire table.
Instant column size changes (in bytes) are not supported by
all storage engines.
Instant CHAR column changes are only allowed for InnoDB
ROW_FORMAT=REDUNDANT. We could allow this for InnoDB
when the CHAR internally uses a variable-length encoding,
say, when converting from 3-byte UTF-8 to 4-byte UTF-8.
Instant VARCHAR column changes are allowed for InnoDB
ROW_FORMAT=REDUNDANT, and for others only if the size
in bytes does not change from 128..255 bytes to more
than 256 bytes.
Inside InnoDB, this slightly changes the way how MDEV-15563
works and fixes the result of the innodb.instant_alter_extend test.
We change the way how ALTER_COLUMN_EQUAL_PACK_LENGTH_EXT
is handled. All column extension, type changes and renaming
now go through a common route, except when ctx->is_instant()
is in effect, for example, instant ADD or DROP COLUMN has
been initiated. Only in that case we will go through
innobase_instant_try() and rewrite all column metadata.
get_type(field, prtype, mtype, len): Convert a SQL data type into
InnoDB column metadata.
innobase_rename_column_try(): Remove the update of SYS_COLUMNS.
innobase_rename_or_enlarge_column_try(): New function,
replacing part of innobase_rename_column_try() and all of
innobase_enlarge_column_try(). Also changes column types.
innobase_rename_or_enlarge_columns_cache(): Also change
the column type.
When creating a field of type JSON, it will be automatically
converted to TEXT with CHECK (json_valid(`a`)), if there wasn't any
previous check for the column.
Additional things:
- Added two bug fixes that was found while testing JSON. These bug
fixes has also been pushed to 10.3 (with a test case), but as they
where minimal and needed to get this task done and tested, the fixes
are repeated here.
- CREATE TABLE ... SELECT drops constraints for columns that
are both in the create and select part.
- If one has both a default expression and check constraint for a
column, one can get the error "Expression for field `a` is refering
to uninitialized field `a`.
- Removed some duplicate MYSQL_PLUGIN_IMPORT symbols
MDEV-17625 Different warnings when comparing a garbage to DATETIME vs TIME
- Splitting processes of data type conversion (to TIME/DATE,DATETIME)
and warning generation.
Warning are now only get collected during conversion (in an "int" variable),
and are pushed in the very end of conversion (not in parallel).
Warnings generated by the low level routines str_to_xxx() and number_to_xxx()
can now be changed at the end, when TIME_FUZZY_DATES is applied,
from "Invalid value" to "Truncated invalid value".
Now "Illegal value" is issued only when the low level routine returned
an error and TIME_FUZZY_DATES was not set. Otherwise, if the low level
routine returned "false" (success), or if NULL was converted to a zero
datetime by TIME_FUZZY_DATES, then "Truncated illegal value"
is issued. This gives better warnings.
- Methods Type_handler::Item_get_date() and
Type_handler::Item_func_hybrid_field_type_get_date() now only
convert and collect warning information, but do not push warnings.
- Changing the return data type for Type_handler::Item_get_date()
and Type_handler::Item_func_hybrid_field_type_get_date() from
"bool" to "void". The conversion result (success vs error) can be
checked by testing ltime->time_type. MYSQL_TIME_{NONE|ERROR}
mean mean error, other values mean success.
- Adding new wrapper methods Type_handler::Item_get_date_with_warn() and
Type_handler::Item_func_hybrid_field_type_get_date_with_warn()
to do conversion followed by raising warnings, and changing
the code to call new Type_handler::***_with_warn() methods.
- Adding a helper class Temporal::Status, a wrapper
for MYSQL_TIME_STATUS with automatic initialization.
- Adding a helper class Temporal::Warn, to collect warnings
but without actually raising them. Moving a part of ErrConv
into a separate class ErrBuff, and deriving both Temporal::Warn
and ErrConv from ErrBuff. The ErrBuff part of Temporal::Warn
is used to collect textual representation of the input data.
- Adding a helper class Temporal::Warn_push. It's used
to collect warning information during conversion, and
automatically pushes warnings to the diagnostics area
on its destructor time (in case of non-zero warning).
- Moving more code from various functions inside class Temporal.
- Adding more Temporal_hybrid constructors and
protected Temporal methods make_from_xxx(),
which convert and only collect warning information, but do not
actually raise warnings.
- Now the low level functions str_to_datetime() and str_to_time()
always set status->warning if the return value is "true" (error).
- Now the low level functions number_to_time() and number_to_datetime()
set the "*was_cut" argument if the return value is "true" (error).
- Adding a few DBUG_ASSERTs to make sure that str_to_xxx() and
number_to_xxx() always set warnings on error.
- Adding new warning flags MYSQL_TIME_WARN_EDOM and MYSQL_TIME_WARN_ZERO_DATE
for the code symmetry. Before this change there was a special
code path for (rc==true && was_cut==0) which was treated by
Field_temporal::store_invalid_with_warning as "zero date violation".
Now was_cut==0 always means that there are no any error/warnings/notes
to be raised, not matter what rc is.
- Using new Temporal_hybrid constructors in combination with
Temporal::Warn_push inside str_to_datetime_with_warn(),
double_to_datetime_with_warn(), int_to_datetime_with_warn(),
Field::get_date(), Item::get_date_from_string(), and a few other places.
- Removing methods Dec_ptr::to_datetime_with_warn(),
Year::to_time_with_warn(), my_decimal::to_datetime_with_warn(),
Dec_ptr::to_datetime_with_warn().
Fixing Sec6::to_time() and Sec6::to_datetime() to
convert and only collect warnings, without raising warnings.
Now warning raising functionality resides in Temporal::Warn_push.
- Adding classes Longlong_hybrid_null and Double_null, to
return both value and the "IS NULL" flag. Adding methods
Item::to_double_null(), to_longlong_hybrid_null(),
Item_func_hybrid_field_type::to_longlong_hybrid_null_op(),
Item_func_hybrid_field_type::to_double_null_op().
Removing separate classes VInt and VInt_op, as they
have been replaced by a single class Longlong_hybrid_null.
- Adding a helper method Temporal::type_name_by_timestamp_type(),
moving a part of make_truncated_value_warning() into it,
and reusing in Temporal::Warn::push_conversion_warnings().
- Removing Item::make_zero_date() and
Item_func_hybrid_field_type::make_zero_mysql_time().
They provided duplicate functionality.
Now this code resides in Temporal::make_fuzzy_date().
The latter is now called for all Item types when data type
conversion (to DATE/TIME/DATETIME) is involved, including
Item_field and Item_direct_view_ref.
This fixes MDEV-17563: Item_direct_view_ref now correctly converts
NULL to a zero date when TIME_FUZZY_DATES says so.
C++ does not guarantee the order of parameter evaluation.
It was wrong to pass item->val_int() and item->null_value
at the same time to any function or constructor.
Adding a new helper class Longlong_null, and new methods
Item::to_longlong_null() and Item_func_hybrid_field_type::to_longlong_null_op(),
which make sure to properly call val_int()/int_op() and test null_value.
Reorganizing the rest of the code accordingly.
The problem happened because {{Field_xxx::store(longlong nr, bool unsigned_val)}} erroneously passed {{unsigned_flag}} to the {{usec}} parameter of this constructor:
{code:cpp}
Datetime(int *warn, longlong sec, ulong usec, date_conv_mode_t flags)
{code}
1. Changing Time and Datetime constructors to accept data as Sec6 rather than as
longlong/double/my_decimal, so it's not possible to do such mistakes
in the future. Additional good effect of these changes:
- This reduced some amount of similar code (minus ~35 lines).
- The code now does not rely on the fact that "unsigned_flag" is
not important inside Datetime().
The constructor always gets all three parts: sign, integer part,
fractional part. The simple the better.
2. Fixing Field_xxx::store() to use the new Datetime constructor format.
This change actually fixes the problem.
3. Adding "explicit" keyword to all Sec6 constructors,
to avoid automatic hidden conversion from double/my_decimal to Sec6,
as well as from longlong/ulonglong through double to Sec6.
4. Change#1 caused (as a dependency) changes in a few places
with code like this:
bool neg= nr < 0 && !unsigned_val;
ulonglong value= m_neg ? (ulonglong) -nr : (ulonglong) nr;
These fragments relied on a non-standard behavior with
the operator "minus" applied to the lowest possible negative
signed long long value. This can lead to different results
depending on the platform and compilation flags.
We have fixed such bugs a few times already.
So instead of modifying the old wrong code to a new wrong code,
replacing all such fragments to use Longlong_hybrid,
which correctly handles this special case with -LONGLONG_MIN
in its method abs().
This also reduced the amount of similar code
(1 or 2 new lines instead 3 old lines in all 6 such fragments).
5. Removing ErrConvInteger(longlong nr, bool unsigned_flag= false)
and adding ErrConvInteger(Longlong_hybrid) instead, to encourage
use of safe Longlong_hybrid instead of unsafe pairs nr+neg.
6. Removing unused ErrConvInteger from Item_cache_temporal::get_date()
Problems:
Functions LEAST() and GREATEST() in TIME context, as well as functions
TIMESTAMP(a,b) and ADDTIME(a,b), returned confusing results when the
input TIME-alike value in a number or in a string was out of the TIME
supported range.
In case of TIMESTAMP(a,b) and ADDTIME(a,b), the second argument
value could get extra unexpected digits. For example, in:
ADDTIME('2001-01-01 00:00:00', 10000000) or
ADDTIME('2001-01-01 00:00:00', '1000:00:00')
the second argument was converted to '838:59:59.999999'
with six fractional digits, which contradicted "decimals"
previously set to 0 in fix_length_and_dec().
These unexpected fractional digits led to confusing function results.
Changes:
1. GREATEST(), LEAST()
- fixing Item_func_min_max::get_time_native()
to respect "decimals" set by fix_length_and_dec().
If a value of some numeric or string time-alike argument
goes outside of the TIME range and gets limited to '838:59:59.999999',
it's now right-truncated to the correct fractional precision.
- fixing, Type_handler_temporal_result::Item_func_min_max_fix_attributes()
to take into account arguments' time_precision() or datetime_precision(),
rather than rely on "decimals" calculated by the generic implementation
in Type_handler::Item_func_min_max_fix_attributes(). This makes
GREATEST() and LEAST() return better data types, with the same
fractional precision with what TIMESTAMP(a,b) and ADDTIME(a,b) return
for the same arguments, and with DATE(a) and TIMESTAMP(a).
2. Item_func_add_time and Item_func_timestamp
It was semantically wrong to apply the limit of the TIME data type
to the argument "b", which plays the role of "INTERVAL DAY TO SECOND" here.
Changing the code to fetch the argument "b" as INTERVAL rather than as TIME.
The low level routine calc_time_diff() now gets the interval
value without limiting to '838:59:59.999999', so in these examples:
ADDTIME('2001-01-01 00:00:00', 10000000)
ADDTIME('2001-01-01 00:00:00', '1000:00:00')
calc_time_diff() gets '1000:00:00' as is. The SQL function result
now gets limited to the supported result data type range
(datetime or time) inside calc_time_diff(), which now calculates
the return value using the real fractional digits that
came directly from the arguments (without the effect of limiting
to the TIME range), so the result does not have any unexpected
fractional digits any more.
Detailed changes in TIMESTAMP() and ADDTIME():
- Adding a new class Interval_DDhhmmssff. It's similar to Time, but:
* does not try to parse datetime format, as it's not needed for
functions TIMESTAMP() and ADDTIME().
* does not cut values to '838:59:59.999999'
The maximum supported Interval_DDhhmmssff's hard limit is
'UINT_MAX32:59:59.999999'. The maximum used soft limit is:
- '87649415:59:59.999999' (in 'hh:mm:ss.ff' format)
- '3652058 23:59:59.999999' (in 'DD hh:mm:ss.ff' format)
which is a difference between:
- TIMESTAMP'0001-01-01 00:00:00' and
- TIMESTAMP'9999-12-31 23:59:59.999999'
(the minimum datetime that supports arithmetic, and the
maximum possible datetime value).
- Fixing get_date() methods in the classes related to functions
ADDTIME(a,b) and TIMESTAMP(a,b) to use the new class Interval_DDhhmmssff
for fetching data from the second argument, instead of get_date().
- Fixing fix_length_and_dec() methods in the classes related
to functions ADDTIME(a,b) and TIMESTAMP(a,b) to use
Interval_DDhhmmssff::fsp(item) instead of item->time_precision()
to get the fractional precision of the second argument correctly.
- Splitting the low level function str_to_time() into smaller pieces
to reuse the code. Adding a new function str_to_DDhhmmssff(), to
parse "INTERVAL DAY TO SECOND" values.
After these changes, functions TIMESTAMP() and ADDTIME()
return much more predictable results, in terms of fractional
digits, and in terms of the overall result.
The full ranges of DATETIME and TIME values are now covered by TIMESTAMP()
and ADDTIME(), so the following can now be calculated:
SELECT ADDTIME(TIMESTAMP'0001-01-01 00:00:00', '87649415:59:59.999999');
-> '9999-12-31 23:59:59.999999'
SELECT TIMESTAMP(DATE'0001-01-01', '87649415:59:59.999999')
-> '9999-12-31 23:59:59.999999'
SELECT ADDTIME(TIME'-838:59:59.999999', '1677:59:59.999998');
-> '838:59:59.999999'
Fixing C++ function check_date() to get the "fuzzydate" as
date_mode_t rather than ulonglong, so conversion from
date_time_t to ulonglong is now done inside C++ check_date(),
and no conversion is needed in the callers' code.
As an additional safety, modified the code not to pass
TIME_FUZZY_DATE to the low level C functions:
- check_date()
- str_to_datetime()
- str_to_time()
- number_to_datetime()
because TIME_FUZZY_DATE is known only on the C++ level,
C functions do not know it.
Soon we'll be adding more flags into the C++ level (i.e. to date_time_t),
e.g. for rounding. It's a good idea to prevent passing C++ specific
flags into pure C routines before this change.
Asserts were added into the affected C functions to verify
that the caller passed only known C level flags.
Also fixes:
MDEV-17330 Wrong result for 0 + LEAST(TIME'-10:00:00',TIME'10:00:00')
Problems:
1. These methods did not take into account the current session date flags
and passed date_mode_t(0) to func->get_date():
Type_handler_temporal_result::Item_func_min_max_val_real
Type_handler_temporal_result::Item_func_min_max_val_int
Type_handler_temporal_result::Item_func_min_max_val_decimal
Fixing to pass sql_mode_for_dates(thd) instead of date_mode_t(0).
Note, sql_mode_for_dates(thd) is only needed for DATE/DATETIME
data types. It is not needed for TIME.
So splitting value methods Type_handler_temporal_result::Item_func_min_max_xxx
into individual implementations for
Type_handler_{time|date|datetime|timestamp}_common
and, instead of calling get_date(), reusing inside classes
Time(), Date(), Datetime() and their methods to_longlong().
sql_mode_for_dates(thd) is automatically passed to get_date()
inside Date() and Datetime() constructors.
The switch to classes also fixed the problem reported in MDEV-17330.
Type_handler_temporal_result::Item_func_min_max_val_int() used to
call TIME_to_ulonglong(), which was not correct for TIME.
Changing the code to use Time().to_longlong() solved this.
2. Type_handler_temporal_result::Item_func_min_max_get_date
also did not take into account the current session
date flags in case of conversion from DATE/DATETIME to time
and passed date_mode_t(0) to get_date_native().
Fixing to pass sql_mode_for_dates(thd) in case of conversion
from DATE/DATETIME to TIME.
