Run till exit from #0  cmp_rec_rec (rec1=rec1@entry=0x7f6820095fb2 ' ' <repeats 24 times>, "\200", rec2=rec2@entry=0x7f68360c8087 ' ' <repeats 24 times>, "\200", offsets1=offsets1@entry=0x7f6820095fe0, 

    offsets2=0x7f6820096000, index=0x7f6820195fd0, nulls_unequal=nulls_unequal@entry=false, matched_fields=0x0) at /mariadb/10.2o/storage/innobase/rem/rem0cmp.cc:1116

0x0000562b9a677c60 in PageBulk::insert (this=this@entry=0x7f6820095960, rec=rec@entry=0x7f6820095fb2 ' ' <repeats 24 times>, "\200", offsets=offsets@entry=0x7f6820095fe0)

    at /mariadb/10.2o/storage/innobase/btr/btr0bulk.cc:199

199			ut_ad(cmp_rec_rec(rec, old_rec, offsets, old_offsets, m_index)

Value returned is $1 = -1

The records that are submitted to comparison are:
(a,id,DB_TRX_ID,DB_ROLL_PTR)=('',1,0x503,(insert)),('',2,0x50f,(insert))

The assertion fails because the field id in old_rec is larger (2) than in rec (1).

We have skip_pk_sort=true, because the table-rebuilding ALTER TABLE can use the pre-existing ordering of PRIMARY KEY(a,id). The ordering was violated already when the second row was inserted. The problem is that two empty CHAR with exactly the same contents are being treated unequal (-32 will be returned) when searching the position for the second INSERT:

#0  my_strnncollsp_utf8_nopad_bin (cs=0x55efb6c27720 <my_charset_utf8_nopad_bin>, a=0x7fdbdc035098 ' ' <repeats 24 times>, "\002", a_length=8, b=0x7fdbf332407e ' ' <repeats 24 times>, "\200", b_length=24)

    at /mariadb/10.2o/strings/strcoll.ic:222

#1  0x000055efb5ece628 in innobase_mysql_cmp (prtype=72548862, a=0x7fdbdc035098 ' ' <repeats 24 times>, "\002", a_length=8, b=0x7fdbf332407e ' ' <repeats 24 times>, "\200", b_length=24)

    at /mariadb/10.2o/storage/innobase/rem/rem0cmp.cc:88

#2  0x000055efb5ecee27 in cmp_whole_field (mtype=13, prtype=72548862, a=0x7fdbdc035098 ' ' <repeats 24 times>, "\002", a_length=8, b=0x7fdbf332407e ' ' <repeats 24 times>, "\200", b_length=24)

    at /mariadb/10.2o/storage/innobase/rem/rem0cmp.cc:374

#3  0x000055efb5ed066c in cmp_data (mtype=13, prtype=72548862, data1=0x7fdbdc035098 ' ' <repeats 24 times>, "\002", len1=8, data2=0x7fdbf332407e ' ' <repeats 24 times>, "\200", len2=24)

    at /mariadb/10.2o/storage/innobase/rem/rem0cmp.cc:449

#4  0x000055efb5ecf838 in cmp_dtuple_rec_with_match_bytes (dtuple=0x7fdbdc024f10, rec=0x7fdbf332407e ' ' <repeats 24 times>, "\200", index=0x7fdbdc0410f0, offsets=0x7fdbe00f10d0, matched_fields=0x7fdbe00f1048, 

    matched_bytes=0x7fdbe00f1050) at /mariadb/10.2o/storage/innobase/rem/rem0cmp.cc:860

#5  0x000055efb5ea1c68 in page_cur_search_with_match_bytes (block=0x7fdbf2e1c480, index=0x7fdbdc0410f0, tuple=0x7fdbdc024f10, mode=PAGE_CUR_LE, iup_matched_fields=0x7fdbe00f18d8, 

    iup_matched_bytes=0x7fdbe00f18e0, ilow_matched_fields=0x7fdbe00f18e8, ilow_matched_bytes=0x7fdbe00f18f0, cursor=0x7fdbe00f2908) at /mariadb/10.2o/storage/innobase/page/page0cur.cc:707

#6  0x000055efb600a1a3 in btr_cur_search_to_nth_level_func (index=0x7fdbdc0410f0, level=0, tuple=0x7fdbdc024f10, mode=PAGE_CUR_LE, latch_mode=2, cursor=0x7fdbe00f2900, has_search_latch=0, 

    file=0x55efb6504b60 "/mariadb/10.2o/storage/innobase/row/row0ins.cc", line=2606, mtr=0x7fdbe00f2c60, autoinc=0) at /mariadb/10.2o/storage/innobase/btr/btr0cur.cc:1650

#7  0x000055efb5ef2c8b in btr_pcur_open_low (index=0x7fdbdc0410f0, level=0, tuple=0x7fdbdc024f10, mode=PAGE_CUR_LE, latch_mode=2, cursor=0x7fdbe00f2900, 

    file=0x55efb6504b60 "/mariadb/10.2o/storage/innobase/row/row0ins.cc", line=2606, autoinc=0, mtr=0x7fdbe00f2c60) at /mariadb/10.2o/storage/innobase/include/btr0pcur.ic:457

#8  0x000055efb5ef9138 in row_ins_clust_index_entry_low (flags=0, mode=2, index=0x7fdbdc0410f0, n_uniq=2, entry=0x7fdbdc024f10, n_ext=0, thr=0x7fdbdc033930)

    at /mariadb/10.2o/storage/innobase/row/row0ins.cc:2605

Note: We are passing b_length=24 and a_length=8 here. One is in bytes, the other in characters.

In ROW_FORMAT=REDUNDANT, InnoDB reserves n*mbmaxlen bytes for each CHAR column. In other ROW_FORMAT, CHAR columns of variable-length encoded character sets occupy n*mbminlen to n*mbmaxlen bytes.

The ‘incorrect’ length of 8 (characters, not bytes) was set here:

#0  0x000055efb5eefc2d in dfield_set_data (field=0x7fdbdc024f48, data=0x7fdbdc035098, len=8) at /mariadb/10.2o/storage/innobase/include/data0data.ic:97

#1  0x000055efb5efb04d in row_ins_index_entry_set_vals (index=0x7fdbdc0410f0, entry=0x7fdbdc024f10, row=0x7fdbdc0337a8) at /mariadb/10.2o/storage/innobase/row/row0ins.cc:3375

#2  0x000055efb5efb125 in row_ins_index_entry_step (node=0x7fdbdc0336c8, thr=0x7fdbdc033930) at /mariadb/10.2o/storage/innobase/row/row0ins.cc:3402

#3  0x000055efb5efb522 in row_ins (node=0x7fdbdc0336c8, thr=0x7fdbdc033930) at /mariadb/10.2o/storage/innobase/row/row0ins.cc:3548

#4  0x000055efb5efb8b2 in row_ins_step (thr=0x7fdbdc033930) at /mariadb/10.2o/storage/innobase/row/row0ins.cc:3672

#5  0x000055efb5f18d25 in row_insert_for_mysql (mysql_rec=0x7fdbdc035098 ' ' <repeats 24 times>, "\002", prebuilt=0x7fdbdc033180) at /mariadb/10.2o/storage/innobase/row/row0mysql.cc:1408

#6  0x000055efb5de3fb0 in ha_innobase::write_row (this=0x7fdbdc027540, record=0x7fdbdc035098 ' ' <repeats 24 times>, "\002") at /mariadb/10.2o/storage/innobase/handler/ha_innodb.cc:8252

The problem was introduced by bar in MariaDB Server 10.2.2 by MDEV-9711 (NO_PAD collations).

An easy fix would be to reject the creation of indexes on NO_PAD collated CHAR columns. A much more risky and costly fix would be to rewrite large parts of InnoDB to take care of this somehow. It could cause a performance regression, and we might still miss some cases.

With some advice from bar, I patched cmp_data() so that it would trim the extra padding. This is only a proof of concept of a fix, and my debug assertions would almost certainly fail for other than ROW_FORMAT=REDUNDANT tables. But, the patch made the test case pass.

I am not sure how to fix this. InnoDB only stores column and index field lengths in bytes.

MDEV-6929 introduced something that would convert a column to bytes, incorrectly (MDEV-12255, MDEV-12486, MDEV-23600).

Unless ROW_FORMAT=REDUNDANT, a CHAR column of a variable-length encoding such as UTF-8 or UTF-16 is actually stored as variable length in InnoDB, occupying n*mbminlen to n*mbmaxlen bytes. For example, the CHAR(3) CHARACTER SET utf8mb3 column value 'äh' would be stored in exactly 3 bytes, using 2 bytes for the first character and 1 byte for the next one. Trailing spaces will be trimmed in order to reach the minimum column value of 3*1 bytes.

At the low level, such as cmp_data_data(), we only have the column lengths in bytes, not in characters at all. How are we supposed to compare CHAR columns in a NO PAD collation in this case? If one of the CHAR(3) strings is 'äh' and another is 'ah ' (note the trailing space), which comparison should be invoked?

What is 'ah '? Shouldn't it be 'ah' followed by seven spaces, to make nine bytes total, where nine is CHAR(3)*3, where the last 3 is mbmaxlen for utf8mb3?

This is what the Field::ptr representation provides.

Sorry, I reread your comment and understood. It's an InnoDB specific representation which is different from Field::ptr representation.

The function added by MDEV-25449 currently does not cover this case. You'll have to extend the values with trailing spaces to be at least 3 characters long before passing them to strnncollsp_nchars().

In the currently MDEV-25549 implementation, strnncollsp_nchars() can only skip redundant trailing spaces. But in cannot pad missing trailing spaces. This will need extra coding.

bar, thank you, and I am sorry for not making this clear in the beginning.
For InnoDB, it would be easiest if we could avoid the conversion from bytes to characters. The truncation of CHAR data happens in the function row_mysql_store_col_in_innobase_format() for other than ROW_FORMAT=REDUNDANT:

	} else if (comp && type == DATA_MYSQL

		   && dtype_get_mbminlen(dtype) == 1

		   && dtype_get_mbmaxlen(dtype) > 1) {

		/* In some cases we strip trailing spaces from UTF-8 and other

		multibyte charsets, from FIXED-length CHAR columns, to save

		space. UTF-8 would otherwise normally use 3 * the string length

		bytes to store an ASCII string! */

At that point, we do know the length of the full column in bytes:

		n_chars = dtype_get_len(dtype) / dtype_get_mbmaxlen(dtype);

But, in rem0cmp.cc we would currently not have that information available. We would have to add an additional parameter to many functions, which would not only be a large change but could also hurt performance. At that level, we basically only know the length of both fields in bytes, the collation code, and that the column is CHAR (mtype==DATA_MYSQL for CHAR encodings and collations other than latin1_swedish_ci).

For InnoDB, it would be easier if we had something that would override the NO PAD attribute of the collation, and just compare the two CHAR index fields as they would be compared without NO PAD. As far as I understand, CHAR should always be logically space-padded (hence NO PAD should make no difference), and we would never compare VARCHAR to CHAR inside InnoDB.

server part is ok as per MDEV-25904.
I don't think I can review InnoDB part.

I think that the InnoDB part can be best covered by Random Query Generator, possibly tweaked to increase the use of nopad collations.

Testing bb-10.4-MDEV-25440 using pquery with a full list of all possible charset and collations as per git grep 'charset_info.*nopad' include/m_ctype.h annd a mixed of REDUNDANT/DYNAMIC/notspecified for every table.

The patches in https://github.com/mariadb/server/tree/bb-10.4-MDEV-25440 are OK to push.

OK to push, no directly related issues observed during testing.

This was not fixed in the 10.2 or 10.3 series, because the fix depends on MDEV-25904 which was only fixed in 10.4 and later.

Additional testing was done on the second iteration of the patch as requested by Marko.

Logged MDEV-27768 MDEV-25440: Assertion `(cs->state & 0x20000) == 0' failed in my_strnncollsp_nchars_generic_8bit