[ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes implementation

[Ben Pfaff]

On Sat, Jun 24, 2017 at 05:01:51PM -0400, Lance Richardson wrote:
> This patch adds support for the creation of multicolumn indexes
> in the C IDL to enable for efficient search and retrieval of database
> rows by key.
> 
> Signed-off-by: Esteban Rodriguez Betancourt <estebarb at hpe.com>
> Co-authored-by: Lance Richardson <lrichard at redhat.com>
> Signed-off-by: Lance Richardson <lrichard at redhat.com>
> ---
>   v5: - Coding style fixes (checkpatch.py)
>       - Fixed memory leak (missing ovsdb_datum_destroy() in
>         ovsdb_idl_index_destroy_row__()).
>       - Some polishing of comment and log message text.

Thanks for reviving this series.

I don't understand ovsdb_idl_index_read().  It is almost the same as
ovsdb_idl_read().  It looks like ovsdb_idl_read() could be implemented
as a wrapper around it, but I'm also not sure why ovsdb_idl_read() can't
be used directly.  Also, I don't understand its comment about "index_set
functions", since there are no functions with index_set in their names.

ovsdb_idl_index_write_() is mostly copy-paste of a part of
ovsdb_idl_txn_write__(), so to avoid code duplication it would be best
to factor that code out of ovsdb_idl_txn_write__() and call it from both
places.

I don't understand the behavior of ovsdb_idl_index_find() and
ovsdb_idl_index_forward_to() for a null 'value' argument.  Is there some
idiomatic usage where the null and nonnull behaviors work out nicely?

The row_sync behavior is confusing.  I remember it slightly from the
previous iteration but I don't remember being convinced it was the best
way to do things.

Thanks,

Ben.