Hi Lars Erik,
I have looked at the patch and although it seems like a "hack" to add
1900 in encodeRecord I think this is a correct approach and gives the
correct result.
One minor question: In decodeRecord you read directly from
field->ptr[0]. Did you consider doing the same (ie. write to ptr[0]) in
encodeRecord?
I have applied the patch and verified that the regression test fails
without the patch and passes with the patch.
Ok to push!
Thanks for fixing this bug.
Olav
Lars-Erik.Bjork@stripped wrote:
> #At file:///home/lb200670/devel/mysql/trampoline/
>
> 2872 lars-erik.bjork@stripped 2008-10-21
> This is a fix for bug#39342 (YEAR '=' comparison fails when index is
> present)
>
> According to the reference documentation, YEARSs can be declared as
> YEAR(2) or YEAR(4) to specify a display width of two or four
> characters. It turns out that YEARs are stored differently in Falcon
> depending on this. In the case of a YEAR(4), when searching
> an index, the search keys given where off by -1900 years, and the
> search did therefore not find the expected result.
> This patch ensures that both types of YEARs are stored equally
> inside Falcon, in a format that matches the search keys given by
> the server. The change is limited to StorageInterface::encodeRecord
> and StorageInterface::decodeRecord.
>
> A regression test is also added as a part of this commit.
> added:
> mysql-test/suite/falcon/r/falcon_bug_39342.result
> mysql-test/suite/falcon/t/falcon_bug_39342.test
> modified:
> storage/falcon/ha_falcon.cpp
>
> === added file 'mysql-test/suite/falcon/r/falcon_bug_39342.result'
> --- a/mysql-test/suite/falcon/r/falcon_bug_39342.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/falcon/r/falcon_bug_39342.result 2008-10-21 08:57:35 +0000
> @@ -0,0 +1,31 @@
> +*** Bug #39342 ***
> +SET @@storage_engine = 'Falcon';
> +DROP TABLE IF EXISTS t1;
> +CREATE TABLE t1 (a_int INT, b_year YEAR) Engine = 'falcon';
> +INSERT INTO t1 values
> +(1989, 1989), (1990, 1990), (1991, 1991), (1992, 1992),
> +(1993, 1993), (1994, 1994), (1995, 1995), (1996, 1996),
> +(1997, 1997), (1998, 1998), (1999, 1999), (2000, 2000);
> +ALTER TABLE t1 ADD INDEX index_year (b_year);
> +SELECT * FROM t1 WHERE b_year < 1992 ORDER BY b_year ASC;
> +a_int b_year
> +1989 1989
> +1990 1990
> +1991 1991
> +SELECT * FROM t1 WHERE b_year > 1992 ORDER BY b_year ASC;
> +a_int b_year
> +1993 1993
> +1994 1994
> +1995 1995
> +1996 1996
> +1997 1997
> +1998 1998
> +1999 1999
> +2000 2000
> +SELECT * FROM t1 WHERE b_year = 1992 ORDER BY b_year ASC;
> +a_int b_year
> +1992 1992
> +SELECT count(*) FROM t1;
> +count(*)
> +12
> +DROP TABLE t1;
>
> === added file 'mysql-test/suite/falcon/t/falcon_bug_39342.test'
> --- a/mysql-test/suite/falcon/t/falcon_bug_39342.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/falcon/t/falcon_bug_39342.test 2008-10-21 08:57:35 +0000
> @@ -0,0 +1,44 @@
> +--source include/have_falcon.inc
> +
> +#
> +# Bug #39342: YEAR '=' comparison fails when index is present
> +#
> +--echo *** Bug #39342 ***
> +
> +# ----------------------------------------------------- #
> +# --- Initialisation --- #
> +# ----------------------------------------------------- #
> +let $engine = 'Falcon';
> +eval SET @@storage_engine = $engine;
> +
> +--disable_warnings
> +DROP TABLE IF EXISTS t1;
> +--enable_warnings
> +
> +# ----------------------------------------------------- #
> +# --- Test --- #
> +# ----------------------------------------------------- #
> +
> +# Four digit YEAR column (default)
> +CREATE TABLE t1 (a_int INT, b_year YEAR) Engine = 'falcon';
> +
> +INSERT INTO t1 values
> +(1989, 1989), (1990, 1990), (1991, 1991), (1992, 1992),
> +(1993, 1993), (1994, 1994), (1995, 1995), (1996, 1996),
> +(1997, 1997), (1998, 1998), (1999, 1999), (2000, 2000);
> +
> +ALTER TABLE t1 ADD INDEX index_year (b_year);
> +
> +SELECT * FROM t1 WHERE b_year < 1992 ORDER BY b_year ASC;
> +SELECT * FROM t1 WHERE b_year > 1992 ORDER BY b_year ASC;
> +SELECT * FROM t1 WHERE b_year = 1992 ORDER BY b_year ASC;
> +
> +# ----------------------------------------------------- #
> +# --- Check --- #
> +# ----------------------------------------------------- #
> +SELECT count(*) FROM t1;
> +
> +# ----------------------------------------------------- #
> +# --- Final cleanup --- #
> +# ----------------------------------------------------- #
> +DROP TABLE t1;
>
> === modified file 'storage/falcon/ha_falcon.cpp'
> --- a/storage/falcon/ha_falcon.cpp 2008-10-16 02:53:35 +0000
> +++ b/storage/falcon/ha_falcon.cpp 2008-10-21 08:57:35 +0000
> @@ -2652,7 +2652,6 @@ void StorageInterface::encodeRecord(ucha
> case MYSQL_TYPE_INT24:
> case MYSQL_TYPE_LONG:
> case MYSQL_TYPE_LONGLONG:
> - case MYSQL_TYPE_YEAR:
> case MYSQL_TYPE_DECIMAL:
> case MYSQL_TYPE_ENUM:
> case MYSQL_TYPE_SET:
> @@ -2660,6 +2659,12 @@ void StorageInterface::encodeRecord(ucha
> dataStream->encodeInt64(field->val_int());
> break;
>
> + case MYSQL_TYPE_YEAR:
> + // Have to use the ptr directly to get the same number for
> + // both two and four digit YEAR
> + dataStream->encodeInt64((int) field->ptr[0]);
> + break;
> +
> case MYSQL_TYPE_NEWDECIMAL:
> {
> int precision = ((Field_new_decimal *)field)->precision;
> @@ -2828,7 +2833,6 @@ void StorageInterface::decodeRecord(ucha
> case MYSQL_TYPE_INT24:
> case MYSQL_TYPE_LONG:
> case MYSQL_TYPE_LONGLONG:
> - case MYSQL_TYPE_YEAR:
> case MYSQL_TYPE_DECIMAL:
> case MYSQL_TYPE_ENUM:
> case MYSQL_TYPE_SET:
> @@ -2837,6 +2841,12 @@ void StorageInterface::decodeRecord(ucha
> ((Field_num*)field)->unsigned_flag);
> break;
>
> + case MYSQL_TYPE_YEAR:
> + // Must add 1900 to give Field_year::store the value it
> + // expects. See also case 'MYSQL_TYPE_YEAR' in encodeRecord()
> + field->store(dataStream->getInt64() + 1900,
> ((Field_num*)field)->unsigned_flag);
> + break;
> +
> case MYSQL_TYPE_NEWDECIMAL:
> {
> int precision = ((Field_new_decimal*) field)->precision;
>
>
>