MDEV-40486 Length check for vector fields in CREATE TABLE ... SELECT - #5520
Conversation
|
|
|
This is an alternative patch to #5443 |
0f93d71 to
884d10b
Compare
abarkov
left a comment
There was a problem hiding this comment.
I rather dislike that VEC_FROMTEXT(NULL) creates a column of the type VECTOR(1). But as far as Serg recommends so, I won't argue.
The patch is OK to push.
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where vector length-limit violations during CREATE TABLE ... SELECT with VEC_FROMTEXT() could bypass earlier checks and end up as assertion failures by adding an earlier length validation in Item_func_vec_fromtext::fix_length_and_dec, and expands coverage with new mysql-test cases.
Changes:
- Switch type inference sizing for
VEC_FROMTEXT()to usemax_char_length()and add an explicitMAX_FIELD_VARCHARLENGTHlimit check infix_length_and_dec. - Add handling for empty/very-short arguments to avoid underflow and avoid inferring zero-dimensional vector columns.
- Extend
mysql-test/main/vector2with regression tests for BLOB/TEXT-derived inputs, boundary sizes, empty strings,[],NULL, and prepared statements.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sql/item_vectorfunc.cc | Adds early vector field-length inference and limit checking for VEC_FROMTEXT() to prevent assertion failures during CTAS/type inference. |
| mysql-test/main/vector2.test | Adds new regression and boundary testcases for VEC_FROMTEXT() length inference and edge cases. |
| mysql-test/main/vector2.result | Updates expected output for the added/changed vector tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The changes of MDEV-39558 2b65294 added length check assertion in Field_varstring constructors, and length check in type inference for SELECT set operations, to emit errors before reaching the assertions. That change caused an error to turn into an assertion failure in a separate path, when the length limit violation is not detected before tripping the assertion. So in this patch we fix it by adding an earlier length check in that path. The reason that we place this check inside Item_func_vec_fromtext::fix_length_and_dec rather than say `create_field_for_create_select is for consistency: If create table t1 as select vec_fromtext(concat('[',group_concat(1),']')) as c1 from seq_1_to_64; fails due to length limit violation, then so should create table t1 (v vector(64) not null); insert into t1 select vec_fromtext(concat('[',group_concat(1),']')) from seq_1_to_64; Also use max_char_length() instead of max_length. This is a more accurate length of characters. And add handling of empty string edge case. Added testcases accordingly. The change that uses max_char_length() causes side effects where creating a table using a VEC_FROMTEXT(CHAR(1)) would result in a 0-dimensional vector field. This is accurate but 0-dim vector table fields should not be allowed. So we make cases like this result in a one-dimensional field. Also fixed the underflow in (args[0]->max_length - 1) * 2 when the arg's max length is 0. Previously this underflow would cause create table t1 select vec_fromtext(NULL) to fail with ER_TOO_BIG_FIELDLENGTH. Now it will be a VECTOR(1) field
884d10b to
bb0ac43
Compare
The changes of MDEV-39558 2b65294 added length check assertion in Field_varstring constructors, and length check in type inference for SELECT set operations, to emit errors before reaching the assertions.
That change caused an error to turn into an assertion failure in a separate path, when the length limit violation is not detected before tripping the assertion. So in this patch we fix it by adding an earlier length check in that path.
The reason that we place this check inside
Item_func_vec_fromtext::fix_length_and_dec rather than say `create_field_for_create_select is for consistency:
If
create table t1 as select
vec_fromtext(concat('[',group_concat(1),']')) as c1 from seq_1_to_64;
fails due to length limit violation, then so should
create table t1 (v vector(64) not null);
insert into t1 select vec_fromtext(concat('[',group_concat(1),']')) from seq_1_to_64;
Also use max_char_length() instead of max_length. This is a more accurate length of characters. And add handling of empty string edge case. Added testcases accordingly.
The change that uses max_char_length() causes side effects where creating a table using a VEC_FROMTEXT(CHAR(1)) would result in a 0-dimensional vector field. This is accurate but 0-dim vector table fields should not be allowed. So we make cases like this result in a one-dimensional field.
Also fixed the underflow in (args[0]->max_length - 1) * 2 when the arg's max length is 0. Previously this underflow would cause
create table t1 select vec_fromtext(NULL)
to fail with ER_TOO_BIG_FIELDLENGTH. Now it will be a VECTOR(1) field