List:Internals« Previous MessageNext Message »
From:Pete Harlan Date:March 9 2007 8:14pm
Subject:Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOB
View as plain text  
Hi,

This is a small patch (to mysql-5.0 bkbits as of yesterday) for a new
SQL mode, called EMPTY_DEFAULT_FOR_BLOB.  Any and all comments are
appreciated.

The idea is this: We would like to enable STRICT_ALL_TABLES, but
because non-null blob/text fields aren't allowed to have a default, we
suddenly have to check all our insert statements to ensure that they
give a default for text/blob fields, even though 100% of the time we
would like the default to be the empty string (as it is without
STRICT_ALL_TABLES.)

Ideally, I'd like a way to specify a default for text/blob fields.
Barring that, this patch adds a mode called EMPTY_DEFAULT_FOR_BLOB
that will silently use '' as the default for text/blob fields, even
with STRICT_ALL_TABLES.

The patch simply prevents MySQL from raising the warning it normally
would (even without STRICT_ALL_TABLES) when it defaults a non-null
blob to the empty string.

A smarter patch might make STRICT_ALL_TABLES not consider *this*
warning an error, but that's beyond how involved a patch I wanted to
write.  And it's reasonable to me that if the user has explicitly
asked for an empty default for BLOB, that it would no longer be
something the user expects a warning for.  So in that regard, I do
think this is the right patch.

In mysql_priv.h, there's a comment that says that if you add another
mode, you should modify a couple of files in the "scripts" directory.
I did that, but it's not included in this patch because the latest
bitkeeper sources don't include the files that mysql_priv.h says to
modify.  (Though the sources as of several days ago did.)

I'd be interested to know if an SQL mode like this could be considered
for mainline inclusion, or what the odds are on someday being allowed
to specify a default for text/blob columns so such a patch would not
be needed.

Thank you for your time,

--Pete

----------------------------------
Pete Harlan
harlan@stripped
http://www.artselect.com
ArtSelect is a subsidiary of a21, Inc.


diff -ur bk.orig/sql/mysql_priv.h bk.pete/sql/mysql_priv.h
--- bk.orig/sql/mysql_priv.h	2007-03-05 02:50:56.000000000 -0800
+++ bk.pete/sql/mysql_priv.h	2007-03-08 15:14:47.000000000 -0800
@@ -410,6 +410,7 @@
 #define MODE_NO_AUTO_CREATE_USER	(MODE_TRADITIONAL*2)
 #define MODE_HIGH_NOT_PRECEDENCE	(MODE_NO_AUTO_CREATE_USER*2)
 #define MODE_NO_ENGINE_SUBSTITUTION     (MODE_HIGH_NOT_PRECEDENCE*2)
+#define MODE_EMPTY_DEFAULT_FOR_BLOB	(MODE_NO_ENGINE_SUBSTITUTION*2)
 /*
   Replication uses 8 bytes to store SQL_MODE in the binary log. The day you
   use strictly more than 64 bits by adding one more define above, you should
diff -ur bk.orig/sql/mysqld.cc bk.pete/sql/mysqld.cc
--- bk.orig/sql/mysqld.cc	2007-03-08 04:27:37.000000000 -0800
+++ bk.pete/sql/mysqld.cc	2007-03-08 15:14:47.000000000 -0800
@@ -226,7 +226,7 @@
   "NO_ZERO_IN_DATE", "NO_ZERO_DATE", "ALLOW_INVALID_DATES",
   "ERROR_FOR_DIVISION_BY_ZERO",
   "TRADITIONAL", "NO_AUTO_CREATE_USER", "HIGH_NOT_PRECEDENCE",
-  "NO_ENGINE_SUBSTITUTION",
+  "NO_ENGINE_SUBSTITUTION", "EMPTY_DEFAULT_FOR_BLOB",
   NullS
 };
 static const unsigned int sql_mode_names_len[]=
@@ -261,7 +261,8 @@
   /*TRADITIONAL*/                 11,
   /*NO_AUTO_CREATE_USER*/         19,
   /*HIGH_NOT_PRECEDENCE*/         19,
-  /*NO_ENGINE_SUBSTITUTION*/      22
+  /*NO_ENGINE_SUBSTITUTION*/      22,
+  /*EMPTY_DEFAULT_FOR_BLOB*/      22
 };
 TYPELIB sql_mode_typelib= { array_elements(sql_mode_names)-1,"",
 			    sql_mode_names,

diff -ur bk.orig/sql/sql_insert.cc bk.pete/sql/sql_insert.cc
--- bk.orig/sql/sql_insert.cc	2007-02-28 23:47:46.000000000 -0800
+++ bk.pete/sql/sql_insert.cc	2007-03-08 15:42:23.000000000 -0800
@@ -1338,12 +1338,17 @@
                                            TABLE_LIST *table_list)
 {
   int err= 0;
+  bool allow_blob_default = 
+      (thd->variables.sql_mode & MODE_EMPTY_DEFAULT_FOR_BLOB);
   for (Field **field=entry->field ; *field ; field++)
   {
     if ((*field)->query_id != thd->query_id &&
         ((*field)->flags & NO_DEFAULT_VALUE_FLAG) &&
         ((*field)->real_type() != FIELD_TYPE_ENUM))
     {
+	if (((*field)->real_type() == FIELD_TYPE_BLOB) && allow_blob_default)
+	    continue;	/* Mode enabled that allows blobs to default to '' */
+
       bool view= FALSE;
       if (table_list)
       {

Thread
Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBPete Harlan9 Mar
  • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBChad MILLER12 Mar
    • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBPete Harlan13 Mar
      • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBChad MILLER13 Mar
        • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBChad MILLER13 Mar
          • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBPete Harlan13 Mar
            • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBChad MILLER13 Mar
              • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBElliot Murphy14 Mar
            • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBStewart Smith14 Mar
            • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBChad MILLER10 Aug
              • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBPete Harlan10 Aug
                • Re: Patch for new SQL mode EMPTY_DEFAULT_FOR_BLOBStewart Smith21 Aug