ovsdb: create column index mapping between ovsdb row to monitor row

Columns indexing is different in ovsdb_row then in ovsdb_monitor_row.
We need mapping between the 2 for condition evaluation.

Signed-off-by: Liran Schour <lirans@il.ibm.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

ovsdb/jsonrpc-server.c

@@ -1158,8 +1158,12 @@ ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor *dbmon,
                 return ovsdb_syntax_error(columns, NULL, "%s is not a valid "
                                           "column name", s);
             }
-            ovsdb_monitor_add_column(dbmon, table, column, select,
+            if (ovsdb_monitor_add_column(dbmon, table, column, select,
-                                     allocated_columns);
+                                         allocated_columns)) {
+                return ovsdb_syntax_error(columns, NULL, "column %s "
+                                          "mentioned more than once",
+                                          column->name);
+            }
         }
     } else {
         struct shash_node *node;
@@ -1167,8 +1171,12 @@ ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor *dbmon,
         SHASH_FOR_EACH (node, &ts->columns) {
             const struct ovsdb_column *column = node->data;
             if (column->index != OVSDB_COL_UUID) {
-                ovsdb_monitor_add_column(dbmon, table, column, select,
+                if (ovsdb_monitor_add_column(dbmon, table, column, select,
-                                         allocated_columns);
+                                             allocated_columns)) {
+                    return ovsdb_syntax_error(columns, NULL, "column %s "
+                                              "mentioned more than once",
+                                              column->name);
+                }
             }
         }
     }
@@ -1217,7 +1225,6 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
 
     SHASH_FOR_EACH (node, json_object(monitor_requests)) {
         const struct ovsdb_table *table;
-        const char *column_name;
         size_t allocated_columns;
         const struct json *mr_value;
         size_t i;
@@ -1251,15 +1258,6 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
                 goto error;
             }
         }
-
-        column_name = ovsdb_monitor_table_check_duplicates(m->dbmon, table);
-
-        if (column_name) {
-            error = ovsdb_syntax_error(mr_value, NULL, "column %s "
-                                       "mentioned more than once",
-                                        column_name);
-            goto error;
-        }
     }
 
     dbmon = ovsdb_monitor_add(m->dbmon);

ovsdb/monitor.c

@@ -116,6 +116,11 @@ struct ovsdb_monitor_table {
     struct ovsdb_monitor_column *columns;
     size_t n_columns;
 
+    /* Columns in ovsdb_monitor_row have different indexes then in
+     * ovsdb_row. This field maps between column->index to the index in the
+     * ovsdb_monitor_row. It is used for condition evaluation. */
+    unsigned int *columns_index_map;
+
     /* Contains 'ovsdb_monitor_changes' indexed by 'transaction'. */
     struct hmap changes;
 };
@@ -304,6 +309,24 @@ ovsdb_monitor_row_destroy(const struct ovsdb_monitor_table *mt,
     }
 }
 
+static void
+ovsdb_monitor_columns_sort(struct ovsdb_monitor *dbmon)
+{
+    int i;
+    struct shash_node *node;
+
+    SHASH_FOR_EACH (node, &dbmon->tables) {
+        struct ovsdb_monitor_table *mt = node->data;
+
+        qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
+              compare_ovsdb_monitor_column);
+        for (i = 0; i < mt->n_columns; i++) {
+            /* re-set index map due to sort */
+            mt->columns_index_map[mt->columns[i].column->index] = i;
+        }
+    }
+}
+
 void
 ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
                                   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
@@ -341,14 +364,21 @@ ovsdb_monitor_add_table(struct ovsdb_monitor *m,
                         const struct ovsdb_table *table)
 {
     struct ovsdb_monitor_table *mt;
+    int i;
+    size_t n_columns = shash_count(&table->schema->columns);
 
     mt = xzalloc(sizeof *mt);
     mt->table = table;
     shash_add(&m->tables, table->schema->name, mt);
     hmap_init(&mt->changes);
+    mt->columns_index_map =
+        xmalloc(sizeof *mt->columns_index_map * n_columns);
+    for (i = 0; i < n_columns; i++) {
+        mt->columns_index_map[i] = -1;
+    }
 }
 
-void
+const char *
 ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
                          const struct ovsdb_table *table,
                          const struct ovsdb_column *column,
@@ -360,39 +390,21 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
 
     mt = shash_find_data(&dbmon->tables, table->schema->name);
 
+    /* Check for column duplication. Return duplicated column name. */
+    if (mt->columns_index_map[column->index] != -1) {
+        return column->name;
+    }
+
     if (mt->n_columns >= *allocated_columns) {
         mt->columns = x2nrealloc(mt->columns, allocated_columns,
                                  sizeof *mt->columns);
     }
 
     mt->select |= select;
+    mt->columns_index_map[column->index] = mt->n_columns;
     c = &mt->columns[mt->n_columns++];
     c->column = column;
     c->select = select;
-}
-
-/* Check for duplicated column names. Return the first
- * duplicated column's name if found. Otherwise return
- * NULL.  */
-const char * OVS_WARN_UNUSED_RESULT
-ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor *m,
-                                     const struct ovsdb_table *table)
-{
-    struct ovsdb_monitor_table *mt;
-    int i;
-
-    mt = shash_find_data(&m->tables, table->schema->name);
-
-    if (mt) {
-        /* Check for duplicate columns. */
-        qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
-              compare_ovsdb_monitor_column);
-        for (i = 1; i < mt->n_columns; i++) {
-            if (mt->columns[i].column == mt->columns[i - 1].column) {
-                return mt->columns[i].column->name;
-            }
-        }
-    }
 
     return NULL;
 }
@@ -1077,6 +1089,8 @@ ovsdb_monitor_add(struct ovsdb_monitor *new_dbmon)
      * connections.  */
     ovs_assert(ovs_list_is_singleton(&new_dbmon->jsonrpc_monitors));
 
+    ovsdb_monitor_columns_sort(new_dbmon);
+
     hash = ovsdb_monitor_hash(new_dbmon, 0);
     HMAP_FOR_EACH_WITH_HASH(dbmon, hmap_node, hash, &ovsdb_monitors) {
         if (ovsdb_monitor_equal(dbmon,  new_dbmon)) {
@@ -1112,6 +1126,7 @@ ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon)
         }
         hmap_destroy(&mt->changes);
         free(mt->columns);
+        free(mt->columns_index_map);
         free(mt);
     }
     shash_destroy(&dbmon->tables);

ovsdb/monitor.h

@@ -52,16 +52,12 @@ void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
 void ovsdb_monitor_add_table(struct ovsdb_monitor *m,
                              const struct ovsdb_table *table);
 
-void ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
+const char * ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
                               const struct ovsdb_table *table,
                               const struct ovsdb_column *column,
                               enum ovsdb_monitor_selection select,
                               size_t *allocated_columns);
 
-const char * OVS_WARN_UNUSED_RESULT
-ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor *,
-                          const struct ovsdb_table *);
-
 struct json *ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon,
                                       bool initial,
                                       uint64_t *unflushed_transaction,