Hi Svoj,
Thanks for the review.
Rafal: please comment on the discussion below.
> > +--source include/not_embedded.inc
> > +--source include/have_federated_db.inc --source
> > +include/have_exampledb.inc
> > +
> Missing have_blackhole.inc.
Ah. Got it.
> > diff -Nrup a/sql/backup/stream_v1.c b/sql/backup/stream_v1.c
> > --- a/sql/backup/stream_v1.c 2008-04-17 06:42:27 -04:00
> > +++ b/sql/backup/stream_v1.c 2008-04-25 11:31:25 -04:00
> > @@ -424,6 +424,7 @@ int bstream_rd_header(backup_stream *s,
> > - 1 = snapshot created by built-in blocking driver (BI_DEFAULT),
> > - 2 = snapshot created using created by built-in driver
> using consistent
> > read transaction (BI_CS).
> > + - 3 = snapshot created by built-in no data driver (BI_NODATA),
> >
> > Format of [backup engine info] depends on snapshot type.
> It is empty for the
> > default and CS snapshots. For native snapshots it has format @@
> > -446,7 +447,8 @@ int bstream_wr_snapshot_info(backup_stre
> > case BI_NATIVE: ret= bstream_wr_byte(s,0); break;
> > case BI_DEFAULT: ret= bstream_wr_byte(s,1); break;
> > case BI_CS: ret= bstream_wr_byte(s,2); break;
> > - default: ret= bstream_wr_byte(s,3); break;
> > + case BI_NODATA: ret= bstream_wr_byte(s,3); break;
> > + default: ret= bstream_wr_byte(s,4); break;
> > }
> >
> How can we enter the "default" case here? I think it is
> impossible, so I'd suggest to either report an error or
> DBUG_ASSERT(0).
You have a point. I am not sure. I recommend we open a new bug on this since
this is part of the original stream work and not something this worklog is
addressing. Shall I open the bug report?
>
>
> > @@ -495,6 +497,7 @@ int bstream_rd_image_info(backup_stream
> > case 0: type= BI_NATIVE; break;
> > case 1: type= BI_DEFAULT; break;
> > case 2: type= BI_CS; break;
> > + case 3: type= BI_NODATA; break;
> > default: return BSTREAM_ERROR;
> > }
> Because at least here it wouldn't be accepted anyway. So why
> do we need to create wittingly broken image file?
Again, I am not sure. I can add this to the bug report too.
> > diff -Nrup a/storage/example/ha_example.cc
> b/storage/example/ha_example.cc
> > --- a/storage/example/ha_example.cc 2008-02-24 08:12:15 -05:00
> > +++ b/storage/example/ha_example.cc 2008-04-25 11:31:27 -04:00
> > @@ -138,6 +138,7 @@ static int example_init_func(void *p)
> > example_hton->state= SHOW_OPTION_YES;
> > example_hton->create= example_create_handler;
> > example_hton->flags= HTON_CAN_RECREATE;
> > + example_hton->db_type= DB_TYPE_EXAMPLE_DB;
> >
> > DBUG_RETURN(0);
> > }
> Let me quote comment from handler.h:
>
> <quot>
> Historical number used for frm file to determine the correct
> storage engine.
> This is going away and new engines will just use "name" for this.
> </quot>
>
> So we're using:
> - obsolete class member, which will be removed some time;
> - possibly uninitialized class member as a storage engine is
> not required to
> initialize it. And example_hton is a great example of that.
>
> My proposal was to add new handlerton flag, but Rafal didn't
> like it. If you really think that flag is not a good idea,
> please use name instead of db_type.
Ok, but name is not a valid method/attribute in the handlerton class. The
server code has over 250 references to db_type so it is clear (to me) that
the switch has yet to occur. Thus, I recommend continue with db_type and
address the issue when/if db_type is removed.
Chuck
