String code and string number work for djikstra and breadth_first, assertion on '0' for no_search

mysql-test/suite/oqgraph/basic.test

 DROP TABLE IF EXISTS graph_base;
 DROP TABLE IF EXISTS graph;
+DROP TABLE IF EXISTS graph2;
 
 CREATE TABLE graph_base (
     from_id INT UNSIGNED NOT NULL,
@@ -8,6 +9,7 @@ CREATE TABLE graph_base (
     INDEX (to_id)
   ) ENGINE=MyISAM;
 
+-- Backwards compat test - should provide a deprecation warning if we do show warnings
 CREATE TABLE graph (
     latch   SMALLINT  UNSIGNED NULL,
     origid  BIGINT    UNSIGNED NULL,
@@ -19,6 +21,18 @@ CREATE TABLE graph (
     KEY (latch, destid, origid) USING HASH
   ) ENGINE=OQGRAPH DATA_TABLE='graph_base' ORIGID='from_id', DESTID='to_id';
 
+CREATE TABLE graph2 (
+    latch   VARCHAR(32) NULL,
+    origid  BIGINT    UNSIGNED NULL,
+    destid  BIGINT    UNSIGNED NULL,
+    weight  DOUBLE    NULL,
+    seq     BIGINT    UNSIGNED NULL,
+    linkid  BIGINT    UNSIGNED NULL,
+    KEY (latch, origid, destid) USING HASH,
+    KEY (latch, destid, origid) USING HASH
+  ) ENGINE=OQGRAPH DATA_TABLE='graph_base' ORIGID='from_id', DESTID='to_id';
+
+
 INSERT INTO graph_base(from_id, to_id) VALUES (1,2), (2,1);
 INSERT INTO graph_base(from_id, to_id) VALUES (1,3), (3,1);
 INSERT INTO graph_base(from_id, to_id) VALUES (3,4), (4,3);
@@ -26,14 +40,44 @@ INSERT INTO graph_base(from_id, to_id) VALUES (5,6), (6,5);
 
 SELECT * FROM graph WHERE latch = 2 AND origid = 1 AND weight = 1;
 
+
+-- expected:
+-- +-------+--------+--------+--------+------+--------+
+-- | latch | origid | destid | weight | seq  | linkid |
+-- +-------+--------+--------+--------+------+--------+
+-- |     2 |      1 |   NULL |      1 |    3 |      3 |
+-- |     2 |      1 |   NULL |      1 |    2 |      2 |
+-- +-------+--------+--------+--------+------+--------+
+-- reset query cache ; flush query cache;
+
+SELECT * FROM graph2 WHERE latch = 'breadth_first' AND origid = 1 AND weight = 1;   -- works
+SELECT * FROM graph2 WHERE latch = '2' AND origid = 1 AND weight = 1;               -- as above
+SELECT * FROM graph2 WHERE latch = 2 AND origid = 1 AND weight = 1;                 -- currently, FAILs - empty set should be as above
+
+SELECT * FROM graph2 WHERE latch='dijkstras' AND origid=1 AND destid=6;
+SELECT * FROM graph2 WHERE latch='1' AND origid=1 AND destid=6;
+SELECT * FROM graph2 WHERE latch='dijkstras' AND origid=1 AND destid=4;
+SELECT * FROM graph2 WHERE latch='1' AND origid=1 AND destid=4;
+SELECT * FROM graph2 WHERE latch='dijkstras' AND origid=4 AND destid=1;
+SELECT * FROM graph2 WHERE latch='1' AND origid=4 AND destid=1;
+
+SELECT * FROM graph2 WHERE latch='no_search' and destid=2 and origid=1;  -- works
+SELECT * FROM graph2 WHERE latch=0 and destid=2 and origid=1;            -- current FAIL - empty set
+SELECT * FROM graph2 WHERE latch='0' and destid=2 and origid=1;          -- causes assertion (at least in debug build, havent tested normal)
+
+SELECT * FROM graph2 WHERE latch=NULL and destid=2 and origid=1;
+
 SELECT * FROM graph WHERE latch = 2 AND origid = 1 AND weight = 2;
 
 SELECT * FROM graph WHERE latch = 2 AND origid = 1 AND (weight = 1 OR weight = 2);
+SELECT * FROM graph2 WHERE latch = 'breadth_first' AND origid = 1 AND (weight = 1 OR weight = 2);
 
 SELECT * FROM graph WHERE latch=1 AND origid=1 AND destid=6;
 SELECT * FROM graph WHERE latch=1 AND origid=1 AND destid=4;
 SELECT * FROM graph WHERE latch=1 AND origid=4 AND destid=1;
 
+
+
 INSERT INTO graph_base (from_id,to_id) VALUES (4,6);
 
 DELETE FROM graph_base WHERE from_id=5;
@@ -42,6 +86,9 @@ DELETE FROM graph_base WHERE from_id=3 AND to_id=5;
 SELECT * FROM graph WHERE latch=1 AND origid=1 AND destid=6;
 SELECT * FROM graph WHERE latch=1 AND origid=6 AND destid=1;
 
+SELECT * FROM graph2 WHERE latch='dijkstras' AND origid=1 AND destid=6;
+SELECT * FROM graph2 WHERE latch='dijkstras' AND origid=6 AND destid=1;
+
 DELETE FROM graph_base;
 FLUSH TABLES;
 TRUNCATE TABLE graph_base;

storage/oqgraph/graphcore.cc

@@ -23,6 +23,7 @@
 */
 
 #include <string.h>
+#include <cstdlib>
 
 #include "graphcore-config.h"
 #include "graphcore-graph.h"
@@ -46,7 +47,7 @@
 using namespace open_query;
 using namespace boost;
 
-static const row empty_row = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+static const row empty_row = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
 
 extern "C" const char* const oqgraph_boost_version= BOOST_LIB_VERSION;
 
@@ -411,11 +412,12 @@ namespace open_query {
 namespace open_query
 {
   inline oqgraph::oqgraph(oqgraph_share *arg) throw()
-    : share(arg), cursor(0)
+    : share(arg), cursor(0), lastRetainedLatch(NULL)
   { }
 
   inline oqgraph::~oqgraph() throw()
   {
+    std::free(lastRetainedLatch);
     delete cursor;
   }
 
@@ -669,6 +671,20 @@ namespace open_query
   }
 #endif
 
+  // THIS IS UGLY - refactor later
+  // Update the retained latch string value, for later retrieval by
+  // fetch_row() as a workaround for making sure we return the correct
+  // string to match the latch='' clause
+  // (This is a hack for mariadb mysql compatibility)
+  // IT SHOULD ONLY BE CALLED IMMEIDATELY BEFORE search)(
+  void oqgraph::retainLatchFieldValue(const char *retainedLatch) 
+  {
+    // attempting to use std::string broke lots of stuff
+    // Probably more efficient to use mysql String class, FIXME later
+    if (lastRetainedLatch) { std::free(lastRetainedLatch); lastRetainedLatch = NULL; }
+    if (retainedLatch) { lastRetainedLatch = strdup(retainedLatch); }
+  }
+
 
   int oqgraph::search(int *latch, VertexID *orig_id, VertexID *dest_id) throw()
   {
@@ -677,8 +693,11 @@ namespace open_query
 
       delete cursor; cursor= 0;
       row_info= empty_row;
-      if ((row_info.latch_indicator= latch))
+      if ((row_info.latch_indicator= latch)) {
         op= ALGORITHM & (row_info.latch= *latch);
+        row_info.latchStringValue = lastRetainedLatch;
+        row_info.latchStringValueLen = strlen(lastRetainedLatch);
+      }
       if ((row_info.orig_indicator= orig_id) && (op|= HAVE_ORIG))
         orig= share->find_vertex((row_info.orig= *orig_id));
       if ((row_info.dest_indicator= dest_id) && (op|= HAVE_DEST))

storage/oqgraph/graphcore.h

@@ -45,6 +45,8 @@ namespace open_query
     bool link_indicator;
 
     int latch;
+    const char* latchStringValue; // workaround for when latch is a Varchar
+    int latchStringValueLen;
     VertexID orig;
     VertexID dest;
     EdgeWeight weight;
@@ -106,6 +108,13 @@ namespace open_query
     int replace_edge(VertexID orig, VertexID dest, EdgeWeight weight) throw()
     { return insert_edge(orig, dest, weight, true); }
 
+    // Update the retained latch string value, for later retrieval by
+    // fetch_row() as a workaround for making sure we return the correct
+    // string to match the latch='' clause
+    // (This is a hack for mariadb mysql compatibility)
+    // IT SHOULD ONLY BE CALLED IMMEIDATELY BEFORE search)(
+    void retainLatchFieldValue(const char *retainedLatch);
+
     int search(int*, VertexID*, VertexID*) throw();
     int random(bool) throw();
 
@@ -120,6 +129,8 @@ namespace open_query
     static void free(oqgraph_share*) throw();
 
     static const size_t sizeof_ref;
+  private:    
+    char *lastRetainedLatch;
   };
 
 }

storage/oqgraph/ha_oqgraph.cc

@@ -694,6 +694,9 @@ int ha_oqgraph::index_next_same(byte *buf, const byte *key, uint key_len)
   return error_code(res);
 }
 
+#define LATCH_WAS CODE 0
+#define LATCH_WAS_NUMBER 1
+
 /**
  * This function parse the VARCHAR(n) latch specification into an integer operation specification compatible with 
  * v1-v3 oqgraph::search().
@@ -707,20 +710,22 @@ int ha_oqgraph::index_next_same(byte *buf, const byte *key, uint key_len)
  * FIXME: For the time being, only handles latin1 character set.
  * @return false if parsing fails.
  */   
-static bool parse_latch_string_to_legacy_int(const String& value, int &latch)
+static int parse_latch_string_to_legacy_int(const String& value, int &latch)
 {
-  // attempt to parse as an integer first.
-  // to be nice we trim whitespace
-  LEX_STRING lex = value.lex_string();
-  trim_whitespace( &my_charset_latin1, &lex);
-  if (!strlen(lex.str)) {
-    // treat an empty string same as (LATCH=NULL) in query
-    latch = oqgraph::NO_SEARCH;
-    return true;
-  }
+  // Attempt to parse as exactly an integer first.
+  
+  // Note: we are strict about not having whitespace, or garbage characters,
+  // so that the query result gets returned properly: 
+  // Because of the way the result is built and used in fill_result,
+  // we have to exactly return in the latch column what was in the latch= clause
+  // otherwise the rows get filtered out by the query optimiser.
+  
+  // For the same reason, we cant simply treat latch='' as NO_SEARCH either.
+  
+  String latchValue = value;
   char *eptr;
-  unsigned long int v = strtoul( lex.str, &eptr, 10);
-  if (!*eptr || eptr == lex.str + lex.length) { // strtoul will 'fail' if non-zero terminated string followed by not 0x0
+  unsigned long int v = strtoul( latchValue.c_ptr_safe(), &eptr, 10);
+  if (!*eptr) {
     // we had an unsigned number. 
     if (v > 0 && v < oqgraph::NUM_SEARCH_OP) {
       latch = v;
@@ -731,7 +736,7 @@ static bool parse_latch_string_to_legacy_int(const String& value, int &latch)
 
   const oqgraph_latch_op_table* entry = latch_ops_table;
   for ( ; entry->key ; entry++) {
-    if (0 == strncmp(entry->key, lex.str, lex.length)) {
+    if (0 == strncmp(entry->key, latchValue.c_ptr_safe(), latchValue.length())) {
       latch = entry->latch;
       return true;
     }
@@ -764,6 +769,7 @@ int ha_oqgraph::index_read_idx(byte * buf, uint index, const byte * key,
     field[2]->move_field_offset(ptrdiff);
   }
 
+  String latchFieldValue;
   if (!field[0]->is_null())
   {
 #ifdef RETAIN_INT_LATCH_COMPATIBILITY
@@ -772,9 +778,8 @@ int ha_oqgraph::index_read_idx(byte * buf, uint index, const byte * key,
     } else 
 #endif
     {
-      String value;
-      field[0]->val_str(&value, &value);
-      if (!parse_latch_string_to_legacy_int(value, latch)) {
+      field[0]->val_str(&latchFieldValue, &latchFieldValue);
+      if (!parse_latch_string_to_legacy_int(latchFieldValue, latch)) {
         // Invalid, so warn & fail
         push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_WRONG_ARGUMENTS, ER(ER_WRONG_ARGUMENTS), "OQGRAPH latch");
         table->status = STATUS_NOT_FOUND;
@@ -804,6 +809,14 @@ int ha_oqgraph::index_read_idx(byte * buf, uint index, const byte * key,
   }
   dbug_tmp_restore_column_map(table->read_set, old_map);
 
+  // Keep the latch around so we can use it in the query result later -
+  // See fill_record().
+  // at the moment our best option is to associate it with the graph
+  // so we pass the string now.
+  // In the future we should refactor parse_latch_string_to_legacy_int()
+  // into oqgraph instead.
+  graph->retainLatchFieldValue(latchFieldValue.c_ptr_safe());
+    
   
   DBUG_PRINT( "oq-debug", ("index_read_idx ::>> search(latch:%s,%ld,%ld)", 
           latchToCode(latch), orig_idp?(long)*orig_idp:-1, dest_idp?(long)*dest_idp:-1));
@@ -812,8 +825,9 @@ int ha_oqgraph::index_read_idx(byte * buf, uint index, const byte * key,
 
   DBUG_PRINT( "oq-debug", ("search() = %d", res));
 
-  if (!res && !(res= graph->fetch_row(row)))
+  if (!res && !(res= graph->fetch_row(row))) {
     res= fill_record(buf, row);
+  }
   table->status = res ? STATUS_NOT_FOUND : 0;
   return error_code(res);
 }
@@ -851,8 +865,7 @@ int ha_oqgraph::fill_record(byte *record, const open_query::row &row)
     field[0]->set_notnull();
     // Convert the latch back to a varchar32
     if (field[0]->type() == MYSQL_TYPE_VARCHAR) {
-      const char *code = latchToCode(row.latch); // Presumably this could be modified to return a String
-      field[0]->store(code, strlen(code), &my_charset_latin1);
+      field[0]->store(row.latchStringValue, row.latchStringValueLen, &my_charset_latin1);
     }
 #ifdef RETAIN_INT_LATCH_COMPATIBILITY
     else if (field[0]->type() == MYSQL_TYPE_SHORT) {
@@ -1080,18 +1093,23 @@ ha_rows ha_oqgraph::records_in_range(uint inx, key_range *min_key,
         // Don't assert, in case the user used alter table on us
         return HA_POS_ERROR;			// Can only use exact keys
       }
-      return graph->vertices_count();
+      unsigned N = graph->vertices_count();
+      DBUG_PRINT( "oq-debug", ("records_in_range ::>> N=%u (vertices)", N));
+      return N;
     }
     return HA_POS_ERROR;			// Can only use exact keys
   }
 
-  if (stats.records <= 1)
+  if (stats.records <= 1) {
+    DBUG_PRINT( "oq-debug", ("records_in_range ::>> N=%u (stats)", stats.records));
     return stats.records;
+  }
 
   /* Assert that info() did run. We need current statistics here. */
   //DBUG_ASSERT(key_stat_version == share->key_stat_version);
   //ha_rows result= key->rec_per_key[key->key_parts-1];
   ha_rows result= 10;
+  DBUG_PRINT( "oq-debug", ("records_in_range ::>> N=%u", result));
 
   return result;
 }