Unable to run UPDATE statement

[abmusse]

Here is a simple example:

CREATE TABLE TESTTABLE (
  FIELD1 char(1) DEFAULT NULL,
  FIELD2 decimal(3,0) DEFAULT NULL
) ENGINE=IBMDB2I;

insert into TESTTABLE (FIELD1, FIELD2) values ('A', 2);

insert into TESTTABLE (FIELD1, FIELD2) values ('A', 2);

update TESTTABLE set FIELD1 = 1;

Error Code: 1031. Storage engine IBMDB2I of the table `mariatest`.`TESTTABLE` doesn't have this option

[kadler]

FYI this error message comes from rc ER_ILLEGAL_HA. We don't set this at all in our code, but it could be getting mapped by MariaDB based on an error we return.

[abmusse]

ha_storage_engine::update_row is the method used for row updates . This can also be seen in the example storage as well.

ERROR 1031 (HY000) translates to ER_ILLEGAL_HA with description Table storage engine for '%s' doesn't have this option.

See https://mariadb.com/kb/en/mariadb-error-codes/.

One interesting tidbit I found while compiling the storage engine is a warning on update_row function (-Woverloaded-virtual):

In file included from ~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/../../sql/sql_show.h:21:0,
                 from ~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/db2i_global.h:62,
                 from ~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/ha_ibmdb2i.h:52,
                 from ~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/ha_ibmdb2i.cc:53:
~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/../../sql/handler.h:4448:15: warning: 'virtual int handler::update_row(const uchar*, const uchar*)' was hidden [-Woverloaded-virtual]
   virtual int update_row(const uchar *old_data __attribute__((unused)),
               ^~~~~~~~~~
In file included from ~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/ha_ibmdb2i.cc:53:0:
~/rpmbuild/BUILD/mariadb-10.3.25/storage/ibmdb2i/ha_ibmdb2i.h:246:7: warning:   by 'int ha_ibmdb2i::update_row(const uchar*, uchar*)' [-Woverloaded-virtual]
   int update_row(const uchar * old_data, uchar * new_data);

Could this warning be related somehow?

[kadler]

Yeah, looks like didn't use the virtual keyword, so the base class doesn't know how to call ours. Instead, it will call the base version which is this:

  virtual int update_row(const uchar *old_data __attribute__((unused)),
                         const uchar *new_data __attribute__((unused)))
  {
      return HA_ERR_WRONG_COMMAND;
  }

I see lots of functions print a warning message using ERR_ILLEGAL_HA when they find HA_ERR_WRONG_COMMAND, eg.

  case HA_ERR_WRONG_COMMAND:
    my_error(ER_ILLEGAL_HA, MYF(0), table_type(), table_share->db.str,
             table_share->table_name.str);
    DBUG_VOID_RETURN;
    break;

[kadler]

I think adding virtual to the beginning of the function definition in our header file will fix it. We should also add override to the end. This will ensure that if the base function changes, we get a compile error since we would no longer be overriding the base class function; that would have caught this error. eg.

virtual int update_row(const uchar * old_data, uchar * new_data) override;

Would be good to add override to the rest of the overridden members functions too.

[abmusse]

Tried adding the above to ha_ibmdb2i.h but got back:

error: virtual int ha_ibmdb2i::update_row(const uchar*, uchar*)' marked 'override', but does not override

~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/ha_ibmdb2i.h:246:15: error: 'virtual int ha_ibmdb2i::update_row(const uchar*, uchar*)' marked 'override', but does not override
   virtual int update_row(const uchar * old_data, uchar * new_data) override;
               ^~~~~~~~~~
In file included from ~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/../../sql/sql_show.h:21:0,
                 from ~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/db2i_global.h:62,
                 from ~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/ha_ibmdb2i.h:52,
                 from ~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/db2i_constraints.cc:36:
~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/../../sql/handler.h:4445:15: warning: 'virtual int handler::update_row(const uchar*, const uchar*)' was hidden [-Woverloaded-virtual]
   virtual int update_row(const uchar *old_data __attribute__((unused)),
               ^~~~~~~~~~
In file included from ~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/db2i_constraints.cc:36:0:
~/rpmbuild/BUILD/mariadb-10.3.29/storage/ibmdb2i/ha_ibmdb2i.h:246:15: warning:   by 'virtual int ha_ibmdb2i::update_row(const uchar*, uchar*)' [-Woverloaded-virtual]
   virtual int update_row(const uchar * old_data, uchar * new_data) override;

[kadler]

Looks like the second parameter should be const uchar* not uchar*. Good thing override caught it.

[abmusse]

Nice! the following resolved the original compile warning shown in #20 (comment)

diff --git a/ha_ibmdb2i.cc b/ha_ibmdb2i.cc
index 41ee101..499a1b3 100644
--- a/ha_ibmdb2i.cc
+++ b/ha_ibmdb2i.cc
@@ -896,7 +896,7 @@ int ha_ibmdb2i::prepareRowForWrite(char* data, char* nulls, bool honorIdentCols)
 
 
 
-int ha_ibmdb2i::update_row(const uchar * old_data, uchar * new_data)
+int ha_ibmdb2i::update_row(const uchar * old_data, const uchar * new_data)
 {
   DBUG_ENTER("ha_ibmdb2i::update_row");
   increment_statistics(&SSV::ha_update_count);
diff --git a/ha_ibmdb2i.h b/ha_ibmdb2i.h
index 9eeb8d5..e6ce96a 100644
--- a/ha_ibmdb2i.h
+++ b/ha_ibmdb2i.h
@@ -243,7 +243,7 @@ public:
   int open(const char *name, int mode, uint test_if_locked);
   int close(void);
   int write_row(uchar * buf);
-  int update_row(const uchar * old_data, uchar * new_data);
+  virtual int update_row(const uchar * old_data, const uchar * new_data) override;
   int delete_row(const uchar * buf);
   int index_init(uint idx, bool sorted);

[kadler]

Did that also fix the update issue?

[abmusse]

Looks like the fix resolves the issue!

MariaDB [(none)]> install plugin ibmdb2i soname "ha_ibmdb2i.so";
Query OK, 0 rows affected (0.021 sec)

MariaDB [(none)]> use test;
Database changed
MariaDB [test]> CREATE TABLE TESTTABLE99 (FIELD1 char(1) DEFAULT NULL,  FIELD2 decimal(3,0) DEFAULT NULL) ENGINE=IBMDB2I;
Query OK, 0 rows affected (0.062 sec)

MariaDB [test]> insert into TESTTABLE99 (FIELD1, FIELD2) values ('A', 2);
Query OK, 1 row affected (0.118 sec)

MariaDB [test]> insert into TESTTABLE99 (FIELD1, FIELD2) values ('A', 2);
Query OK, 1 row affected (0.068 sec)

MariaDB [test]> update TESTTABLE99 set FIELD1 = 1;
Query OK, 2 rows affected (0.029 sec)
Rows matched: 2  Changed: 2  Warnings: 0

MariaDB [test]> SELECT * FROM TESTTABLE99;
+--------+--------+
| FIELD1 | FIELD2 |
+--------+--------+
| 1      |      2 |
| 1      |      2 |
+--------+--------+
2 rows in set (0.001 sec)