Field_blob should only ever have packlength of 4

[Stewart Smith]

for example, the case 3 below should *never* be being called.

   65 : void Field_blob::store_length(unsigned char *i_ptr,
      66 : uint32_t i_packlength,
      67 : uint32_t i_number,
      68 26081 : bool low_byte_first)
      69 : {
      70 : #ifndef WORDS_BIGENDIAN
      71 : (void)low_byte_first;
      72 : #endif
      73 26081 : switch (i_packlength) {
      74 : case 1:
      75 0 : i_ptr[0]= (unsigned char) i_number;
      76 0 : break;
      77 : case 2:
      78 : #ifdef WORDS_BIGENDIAN
      79 : if (low_byte_first)
      80 : {
      81 : int2store(i_ptr,(unsigned short) i_number);
      82 : }
      83 : else
      84 : #endif
      85 0 : shortstore(i_ptr,(unsigned short) i_number);
      86 0 : break;
      87 : case 3:
      88 3 : int3store(i_ptr,i_number);
      89 3 : break;

Further investigation shows this snippet from union.test as the culprit:

SELECT @tmp_max:= @@max_allowed_packet;
SET max_allowed_packet=25000000;
CREATE TABLE t1 (a mediumtext);
CREATE TABLE t2 (b varchar(20));
INSERT INTO t1 VALUES ('a');
CREATE TABLE t3 SELECT REPEAT(a,20000000) AS a FROM t1 UNION SELECT b FROM t2;
--replace_regex /ENGINE=[a-zA-Z]+/ENGINE=DEFAULT/
SHOW CREATE TABLE t3;
DROP TABLES t1,t2,t3;

[Stewart Smith (stewart)]

attached branch has fix (remove all the special length handling code!)

[Stewart Smith (stewart)]

<lbieber> stewart: has this made it to trunk? https://bugs.launchpad.net/drizzle/+bug/494261
* stewart looking
<stewart> lbieber, ahh... that depends on fixing create_internal_temporary_table
<lbieber> stewart: ok.....
<stewart> lbieber, because the code is utter ass. part of the internal temporary table code casts Field_blob to an unrelated type and reads a value from it.
--> MarkAtwood (~matwood@2002:ada0:809d:1:21e:52ff:fe7f:fab3) has joined #drizzle
<stewart> lbieber, and it just so happens that the member of Field_blob that I remove in that patch, is where the temp table code is reading from.
 and it just so happens that the value there kinda makes sense.
 so that bug fix depends on the blueprint of fix internal temporary tables.

[Stewart Smith (stewart)]

Not sure if this is going to make cherry... depends on create internal temporary table rewrite.

[Lee Bieber (kalebral-deactivatedaccount)]

Latest note from Stewart:

if the patch is added, it exposes a bug in code around internal temporary tables that is rather non trivial.
and the replacing of create_internal_temporary_table is probably needed before this fix can be applied

[Brian Aker (brianaker)]

What is issue with internal temp table?

On Aug 3, 2010, at 9:52 AM, Lee Bieber wrote:

> Latest note from Stewart:
>
> if the patch is added, it exposes a bug in code around internal temporary tables that is rather non trivial.
> and the replacing of create_internal_temporary_table is probably needed before this fix can be applied
>
> ** Changed in: drizzle/cherry
> Milestone: ongoing => None
>
> --
> Field_blob should only ever have packlength of 4
> https://bugs.launchpad.net/bugs/494261
> You received this bug notification because you are a member of Drizzle-
> developers, which is subscribed to Drizzle.
>
> Status in A Lightweight SQL Database for Cloud and Web: Fix Committed
> Status in Drizzle cherry series: Fix Committed
>
> Bug description:
> for example, the case 3 below should *never* be being called.
>
> 65 : void Field_blob::store_length(unsigned char *i_ptr,
> 66 : uint32_t i_packlength,
> 67 : uint32_t i_number,
> 68 26081 : bool low_byte_first)
> 69 : {
> 70 : #ifndef WORDS_BIGENDIAN
> 71 : (void)low_byte_first;
> 72 : #endif
> 73 26081 : switch (i_packlength) {
> 74 : case 1:
> 75 0 : i_ptr[0]= (unsigned char) i_number;
> 76 0 : break;
> 77 : case 2:
> 78 : #ifdef WORDS_BIGENDIAN
> 79 : if (low_byte_first)
> 80 : {
> 81 : int2store(i_ptr,(unsigned short) i_number);
> 82 : }
> 83 : else
> 84 : #endif
> 85 0 : shortstore(i_ptr,(unsigned short) i_number);
> 86 0 : break;
> 87 : case 3:
> 88 3 : int3store(i_ptr,i_number);
> 89 3 : break;
>
>
> Further investigation shows this snippet from union.test as the culprit:
>
> SELECT @tmp_max:= @@max_allowed_packet;
> SET max_allowed_packet=25000000;
> CREATE TABLE t1 (a mediumtext);
> CREATE TABLE t2 (b varchar(20));
> INSERT INTO t1 VALUES ('a');
> CREATE TABLE t3 SELECT REPEAT(a,20000000) AS a FROM t1 UNION SELECT b FROM t2;
> --replace_regex /ENGINE=[a-zA-Z]+/ENGINE=DEFAULT/
> SHOW CREATE TABLE t3;
> DROP TABLES t1,t2,t3;
>
>

[Stewart Smith (stewart)]

On Tue, 03 Aug 2010 17:56:11 -0000, Brian Aker <email address hidden> wrote:
> What is issue with internal temp table?

It does some rather invalid casts that just happen to work. You get a
valgrind warning on uninit memory access.

Looking over it today may not be a dumb idea if you're up to feeling
sickened by code...

--
Stewart Smith