Add Select operation#399
Conversation
halfcrazy
force-pushed
the
select-refact
branch
3 times, most recently
from
2ec3aac to
cc49b62
Compare
halfcrazy
force-pushed
the
select-refact
branch
3 times, most recently
from
15ac35a to
f218b18
Compare
|
ping @dave-tucker |
|
Hello @halfcrazy Sorry with the delay. I had this PR on my radar and I think it's important so thanks for the effort. I have just been extremely busy but I will try to keep up actively reviewing it. Before getting into the weeds and looking into the code, I would like to discuss some of the design decisions reflected or that would need to be reflected in the PR OP message :
From the operation perspective on the protocol, no operation supports I don't see any reason this wouldn't work for
I understand how Again this is how all
You don't mention in the PR OP message as this being limited, but in practicality it would be limited for the same reasons 4.. I think trying to keep operations and results as abstract to the user as we can is best. Having the user assume that a select translates to a single operation of known index is something that is already not true for other operations and would not allow us to do what I explained in (1). I would rather have something akin to the existing I would also update the README reflecting how to use this new op. |
Thank you for reviewing. You're right. Because the transact operation is atomic, in theory, we can convert |
|
About result parsing, since the model is already fixed in the The revised function signature would be: Usage Example: var multiCondBridges []bridgeType
multiConditions := []model.Condition{
{
Field: &bridgeModelInstance.Name, // Condition 1: Name == "br-sel1"
Function: ovsdb.ConditionEqual,
Value: bridgeName1,
},
{
Field: &bridgeModelInstance.ExternalIDs, // Condition 2: external_ids includes {"type": "main"}
Function: ovsdb.ConditionIncludes,
Value: map[string]string{"type": "main"},
},
}
selectOpMulti, err := selectClient.WhereAny(&bridgeType{}, multiConditions...).Select()
replyMulti, err := selectClient.Transact(ctx, selectOpMulti...)
_, err = ovsdb.CheckOperationResults(replyMulti, selectOpMulti)
err = selectClient.ParseSelectResult(replyMulti, &multiCondBridges)Key Changes:
Let me know if you need further refinements! |
halfcrazy
force-pushed
the
select-refact
branch
2 times, most recently
from
791e945 to
e7f1029
Compare
|
What happens if there is multiple select operations in the same transaction on the same table? One thing is to aggregate the results of a single select operation that spans multiple operations, but I don't see that as the same thing than as aggregating the results of multiple select operations. Or am I misunderstanding your intentions? |
I see what you mean; indeed, there is a problem with the aggregation logic here. Aggregation should not be done by default for results of multiple queries, other than those introduced by WhereAny. That's a pain, and it seems the only way to do this aggregation is for the user to do it themselves. Any suggestions? One solution I came up with was to add the restriction to check that the query's result requirements are aggregatable by having GetSelectResults additionally receive raw operations. If it doesn't, the user should split it into multiple transaction commits. |
I intended that to be on my original suggestion but I failed, so let me rewrite it:
However, even like this, I am unsure if we would need or not something else to be able to correlate distinct select operations within a transacted bundle. Maybe we will need to add a private hint to the Operation structure (for example, a disctinct uuid for each higher level op) that would otherwise not be serialized over the protocol but still used for the correlation. I don't think the user can do this, because the user in principle has no idea how the higher level op is mapped into lower level OPs. |
Since we support edit： I've reconsidered. If we don't aggregate the results of |
|
So my first suggestion is for the user to pass all operations submitted to transact along with all results obtained from transact and get back a two dimensional array with the (whereany aggregated) results of each select operation. Let's focus on this option first. |
|
update api
// Select generates the OVSDB select operation based on the condition.
// It determines the target table and columns from the condition context.
// Returns an error if the condition was built using WhereCache.
// It also returns a unique correlation ID that can be used to associate
// these operations with their results.
Select(columns ...string) ([]ovsdb.Operation, string, error)
client
// GetSelectResults parses the results of a transaction containing select operations
// and populates the target slices. The targets map is keyed by the correlation ID
// returned from the Select operation. The value is a pointer to a slice of models.
GetSelectResults(ops []ovsdb.Operation, results []ovsdb.OperationResult, targets map[string]interface{}) error |
halfcrazy
force-pushed
the
select-refact
branch
6 times, most recently
from
24e6188 to
fa52d3b
Compare
|
https://github.com/ovn-kubernetes/libovsdb/actions/runs/16991264909/job/48171529192?pr=399#step:4:70 |
halfcrazy
force-pushed
the
select-refact
branch
2 times, most recently
from
7e8bbe1 to
973f882
Compare
Yep, got a fix for that in #424. |
jcaamano
left a comment
jcaamano
left a comment
There was a problem hiding this comment.
Getting there.
Also missing some kind of resolution for
#399 (comment)
#399 (comment)
halfcrazy
force-pushed
the
select-refact
branch
3 times, most recently
from
2e3dead to
1fa82ae
Compare
| // Single pass to find and collect results for the target index | ||
| var selectedResults []ovsdb.OperationResult | ||
| var currentCorrelationID string | ||
| var correlationIDCount int | ||
| var foundTargetGroup bool | ||
|
|
||
| for i, op := range ops { | ||
| correlationID := ovsdb.GetCorrelationID(op) | ||
|
|
||
| if op.Op == ovsdb.OperationSelect && op.Table == targetTable { | ||
| // Check if correlation ID changed | ||
| if correlationID != currentCorrelationID { | ||
| // If we've already found our target group and the correlation ID changes, we're done | ||
| if foundTargetGroup { | ||
| break | ||
| } | ||
|
|
||
| // Update current correlation ID | ||
| currentCorrelationID = correlationID | ||
|
|
||
| // If this is our target index, start collecting results | ||
| if correlationIDCount == index { | ||
| foundTargetGroup = true | ||
| selectedResults = []ovsdb.OperationResult{results[i]} | ||
| } | ||
| correlationIDCount++ | ||
| } else if foundTargetGroup { | ||
| // Same correlation ID and we're in the target group, collect this result | ||
| selectedResults = append(selectedResults, results[i]) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check if we found any results | ||
| if !foundTargetGroup { | ||
| if correlationIDCount == 0 { | ||
| // No select operations found for the target table | ||
| return nil | ||
| } | ||
| return fmt.Errorf("index %d is out of range: found %d query groups for table '%s'", | ||
| index, correlationIDCount, targetTable) | ||
| } | ||
|
|
||
| // Create a map to store merged results (deduplicated by UUID) | ||
| mergedRows := make(map[string]reflect.Value) | ||
|
|
||
| for _, result := range selectedResults { | ||
| if result.Error != "" { | ||
| return fmt.Errorf("operation error: %s: %s", result.Error, result.Details) | ||
| } | ||
|
|
||
| for _, rowData := range result.Rows { | ||
| newModelVal := reflect.New(modelType) | ||
| newModel := newModelVal.Interface().(model.Model) | ||
|
|
||
| info, err := dbModel.NewModelInfo(newModel) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get model info: %w", err) | ||
| } | ||
|
|
||
| if err := dbModel.Mapper.GetRowDataWithUUID(&rowData, info); err != nil { | ||
| return fmt.Errorf("failed to convert row to model: %w", err) | ||
| } | ||
|
|
||
| uuid, err := info.FieldByColumn("_uuid") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get UUID from model: %w", err) | ||
| } | ||
| // Deduplicate by UUID - later results overwrite earlier ones | ||
| mergedRows[uuid.(string)] = newModelVal | ||
| } | ||
| } | ||
|
|
||
| // Populate the target slice with optimized memory allocation | ||
| resultCount := len(mergedRows) | ||
|
|
||
| // Pre-allocate slice with exact capacity to avoid repeated allocations | ||
| if sliceVal.IsNil() || sliceVal.Cap() < resultCount { | ||
| sliceVal.Set(reflect.MakeSlice(sliceVal.Type(), resultCount, resultCount)) | ||
| } else { | ||
| // Reuse existing slice but set to exact length | ||
| sliceVal.SetLen(resultCount) | ||
| } | ||
|
|
||
| // Use index-based assignment to avoid append overhead | ||
| i := 0 | ||
| for _, modelVal := range mergedRows { | ||
| if isPtr { | ||
| sliceVal.Index(i).Set(modelVal) | ||
| } else { | ||
| sliceVal.Index(i).Set(reflect.Indirect(modelVal)) | ||
| } | ||
| i++ | ||
| } |
There was a problem hiding this comment.
What about this:
// Create a map to store merged rows (deduplicated by UUID)
mergedRows := make(map[string]ovsdb.Row)
mergeRows := func(result ovsdb.OperationResult) error {
if result.Error != "" {
return fmt.Errorf("operation error: %s: %s", result.Error, result.Details)
}
for _, row := range result.Rows {
uuid := row["_uuid"]
if uuid == nil {
return fmt.Errorf("failed to get UUID from row: %v", row)
}
// Deduplicate by UUID - later results overwrite earlier ones
// Note different results may have different selected columns
mergedRows[uuid.(string)] = row
}
return nil
}
// Single pass to find and collect results for the target index
currentIndex := -1
var currentCorrelationID string
for i, op := range ops {
if op.Op != ovsdb.OperationSelect || op.Table != targetTable {
continue
}
correlationID := ovsdb.GetCorrelationID(op)
if correlationID != currentCorrelationID {
currentIndex++
currentCorrelationID = correlationID
}
if currentIndex < index {
continue
}
if currentIndex > index {
break
}
err := mergeRows(results[i])
if err != nil {
return err
}
}
if currentIndex < index {
return fmt.Errorf("index %d is out of range: found %d query groups for table '%s'",
index, currentIndex, targetTable)
}
// Populate the target slice with optimized memory allocation
resultCount := len(mergedRows)
// Pre-allocate slice with exact capacity to avoid repeated allocations
if sliceVal.IsNil() || sliceVal.Cap() < resultCount {
sliceVal.Set(reflect.MakeSlice(sliceVal.Type(), resultCount, resultCount))
} else {
// Reuse existing slice but set to exact length
sliceVal.SetLen(resultCount)
}
// Use index-based assignment to avoid append overhead
var i int
for uuid, row := range mergedRows {
model, err := model.CreateModel(dbModel, targetTable, &row, uuid)
if err != nil {
return fmt.Errorf("failed to create model: %w", err)
}
sliceVal.Index(i).Set(reflect.ValueOf(model))
i++
}
| return nil, fmt.Errorf("empty model conditions are not supported for data modification operations (Update, Delete, Mutate). Use specific conditions to target the rows you want to modify") | ||
| } | ||
|
|
||
| func (c *zeroValueConditional) Matches() (map[string]model.Model, error) { |
There was a problem hiding this comment.
I didn't realize this before but this is yet another quirk:
Where(nonZeroModel).Select() will lookup the cache and do a select for the results found on the cache
Where(zeroModel).Select() will not lookup on the cache
The other quirk is that Where(zeroModel).List() will error, and Where(zeroModel).Select() will do something.
That puts me again thinking on the SelectAll option you suggested before. If we did that, then our behavior across all WhereXXX().XXXX() would be consistent and we would require less changes on the conditional API implementation. Whereas SelectAll(model, columns) would require a specific implementation but at least the only user facing quirk is that we would document that model is not used to index. I am now inclined on that last option, sorry for the back and forth. Thoughts?
There was a problem hiding this comment.
Where(nonZeroModel).Select() will lookup the cache and do a select for the results found on the cache
We can translate the index-based condition to an equal condition. And don't use cache for consistency.
The other quirk is that Where(zeroModel).List() will error, and Where(zeroModel).Select() will do something.
Acknowledged. This will be different.
I thought SelectAll(model, columns) is clearer than the internal conditional solution.
There was a problem hiding this comment.
We can translate the index-based condition to an equal condition. And don't use cache for consistency.
We had a use case for using the cache: when you are not caching all columns but you still want to rely on the cache to resolve indexing. We thought of this just in the context of WhereCache but is equally valid to the other WhereXXX methods except Where(model) where you are already proving indexes but even so I think consistency would be using the cache in allo cases with Where(model) as well.
I thought
SelectAll(model, columns)is clearer than the internal conditional solution.
OK! let's try this out.
| // Select returns the operations to search on the database. | ||
| // Depending on the Condition, it might return one or many operations. | ||
| // Use GetSelectResults on the results of the transaction to gather the found Models | ||
| Select(columns ...string) ([]ovsdb.Operation, error) |
There was a problem hiding this comment.
Did you consider to use pointer to fields instead of strings for the columns as we use in other apis (i.e. Update api)? I am not really sure why it was done this way, one reason I can imagine is that the column names is not problematically available for the user through the model so they would need to define their own constants?
There was a problem hiding this comment.
Note, when I implement this, looks still needs some modification to the condition to store model info since select dont require a model param.
https://github.com/ovn-kubernetes/libovsdb/pull/399/files#diff-9c46035c253e42f64c0821dd8d5dc155c0efc59a3ddc51178792da3e62e5bc63R25-R30
// ModelProvider is an optional interface that Conditional implementations
// can implement to provide the model instance used for field pointer resolution.
type ModelProvider interface {
GetModel() model.Model
}
func (c *equalityConditional) GetModel() model.Model {
if len(c.models) > 0 {
return c.models[0]
}
return nil
}
func (c *explicitConditional) GetModel() model.Model {
return c.model
}
func (c *predicateConditional) GetModel() model.Model {
return c.model
}
0a89b96 to
8617cea
Compare
|
|
||
| The `SelectAll` function provides a clear and explicit way to select all rows from a table. The model instance is used only to determine the target table. | ||
|
|
||
| ```go |
There was a problem hiding this comment.
I am getting a MD046 markdown style warning
https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md046.md
I think it claims to use consistent indented code excerpts throughout the document rather thank mixing indented and fenced excerpts.
Multiple occurrences
| ls := &MyLogicalSwitch{} | ||
| selectOps, err := ovs.Where(&MyLogicalSwitch{Name: "sw1"}).Select(&ls.Name, &ls.Ports) |
There was a problem hiding this comment.
nit:
Would it make more sense:
ls := &MyLogicalSwitch{Name: "sw1"}
selectOps, err := ovs.Where(ls).Select(&ls.Name, &ls.Ports)
| #### Processing Select Results with GetSelectResults and GetSelectResultsByIndex | ||
|
|
||
| **GetSelectResults** processes the results from the first select query (most common case): | ||
| ```go | ||
| func (c Client) GetSelectResults(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}) error | ||
| ``` | ||
|
|
||
| **GetSelectResultsByIndex** allows you to choose which select query results to retrieve when you have multiple separate calls to `Select()` for the same model/table: | ||
| ```go | ||
| // The index parameter specifies which select query to retrieve (0-based). | ||
| // Use index=0 for single select queries (WhereAny, WhereCache, etc.). | ||
| func (c Client) GetSelectResultsByIndex(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}, index int) error | ||
| ``` | ||
|
|
||
| **Parameters:** | ||
| - `ops`: The operations that were sent to `Transact` | ||
| - `results`: The results returned from `Transact` | ||
| - `target`: A pointer to a slice of model pointers (e.g., `*[]*MyLogicalSwitch`) | ||
| - `index`: Which select query to retrieve (0 = first `Select()` call, 1 = second, etc.) | ||
|
|
||
| **Understanding the Index Parameter:** | ||
|
|
||
| The `index` parameter refers to the **order of separate `Select()` calls** for the same model/table, not individual operations within a single query: | ||
|
|
||
| ```go | ||
| // Example: Multiple separate Select() calls for the same model | ||
| var ptrResults1, ptrResults2 []*MyLogicalSwitch // Target must be a pointer slice | ||
|
|
||
| // First Select() call - creates select query #0 | ||
| selectOps1, _ := ovs.Where(&MyLogicalSwitch{Name: "sw1"}).Select() | ||
|
|
||
| // Second Select() call - creates select query #1 | ||
| selectOps2, _ := ovs.Where(&MyLogicalSwitch{Name: "sw2"}).Select() | ||
|
|
||
| // Combine operations for a single transaction | ||
| allOps := append(selectOps1, selectOps2...) | ||
| reply, _ := ovs.Transact(ctx, allOps...) | ||
|
|
||
| // Get results from first Select() call (index 0) | ||
| err = ovs.GetSelectResults(allOps, reply, &ptrResults1) | ||
| err = ovs.GetSelectResultsByIndex(allOps, reply, &ptrResults1, 0) // Explicit index 0 | ||
|
|
||
| // Get results from second Select() call (index 1) | ||
| err = ovs.GetSelectResultsByIndex(allOps, reply, &ptrResults2, 1) | ||
| ``` | ||
|
|
||
| **Important Notes:** | ||
| - `WhereAny()` and `WhereCache()` may generate multiple operations, but they all belong to a **single** select query (index 0) | ||
| - Only **separate calls** to `Select()` create different query indices | ||
| - Use `GetSelectResults()` for most cases (single select query) | ||
| - Use `GetSelectResultsByIndex()` only when combining multiple select queries in one transaction | ||
|
|
There was a problem hiding this comment.
nit
I would simplify this section a bit. It looks over-documented when compared with the rest of the README and assumes pre-existing knowledge of implementation details by the user. What about leaving it just at:
#### Processing Select Results with GetSelectResults and GetSelectResultsByIndex
Multiple `Select()` operations can be bundled within a transaction.
**GetSelectResults** retrieves the results for the first select operation.
**GetSelectResultsByIndex** allows you to choose which select operation to retrieve the results for:
```go
// Example: Multiple separate Select() calls for the same model
var ptrResults1, ptrResults2 []*MyLogicalSwitch // Target must be a pointer slice
// First Select() call - creates select query #0
selectOps1, _ := ovs.Where(&MyLogicalSwitch{Name: "sw1"}).Select()
// Second Select() call - creates select query #1
selectOps2, _ := ovs.Where(&MyLogicalSwitch{Name: "sw2"}).Select()
// Combine operations for a single transaction
allOps := append(selectOps1, selectOps2...)
reply, _ := ovs.Transact(ctx, allOps...)
// Get results from first Select() call (index 0)
err = ovs.GetSelectResults(allOps, reply, &ptrResults1)
err = ovs.GetSelectResultsByIndex(allOps, reply, &ptrResults1, 0) // Explicit index 0
// Get results from second Select() call (index 1)
err = ovs.GetSelectResultsByIndex(allOps, reply, &ptrResults2, 1)
|
|
||
| // getColumnsToSelect is a helper function that determines which columns to select | ||
| // based on a model and a list of field pointers. | ||
| func (a api) getColumnsToSelect(m model.Model, tableSchema *ovsdb.TableSchema, tableName string, fields ...any) ([]string, error) { |
There was a problem hiding this comment.
This could be only
func (a api) getColumnsToSelect(m model.Model, fields ...any) ([]string, error) {
as this validation
if _, ok := tableSchema.Columns[colName]; !ok && colName != "_uuid" {
return nil, mapper.NewErrColumnNotFound(colName, tableName)
}
can't possibly fail as this would fail before that?
colName, err := info.ColumnByPtr(field)
if err != nil {
return nil, fmt.Errorf("failed to get column name for field pointer: %w", err)
}
There was a problem hiding this comment.
nit: consider a more generic approach for this method:
func (a api) getColumns(m model.Model, fields ...any) ([]string, error) {
if len(fields) == 0 {
return nil, nil
}
info, err := a.cache.DatabaseModel().NewModelInfo(m)
if err != nil {
return nil, fmt.Errorf("failed to create model info for select: %w", err)
}
return mapper.GetColumns(info, fields) // move remaining implementation to mapper
}
There was a problem hiding this comment.
mapper.GetColumns or info.GetColumns?
There was a problem hiding this comment.
Will mutate update wait using func (a api) getColumns(m model.Model, fields ...any) ([]string, error) ? It doesn't expose mutable/immutable information.
There was a problem hiding this comment.
mapper.GetColumnsorinfo.GetColumns?
I guess info might work better yeah, maybe something like
func (i *Info) ColumnsByPtr(fieldPtrs ...any) ([]string, error) {
columns := make([]string, len(fieldPtrs))
for i, fieldPtr := range fieldPtrs {
columns[i], err := i.ColumnByPtr(fieldPtr)
if err != nil {
return nil, err
}
}
return columns, nil
}
Will mutate update wait using
func (a api) getColumns(m model.Model, fields ...any) ([]string, error)? It doesn't expose mutable/immutable information.
I wasn't thinking on mutate,update or wait using this function, just select for now. But a generic approach makes it readily available for future uses.
| } | ||
|
|
||
| // SelectAll selects all rows from a table, with optional column filtering. | ||
| func (a api) SelectAll(m model.Model, fields ...any) ([]ovsdb.Operation, error) { |
There was a problem hiding this comment.
I would rename to Select and use func (a api) Select(m model.Model, fields ...any) ([]ovsdb.Operation, error) { and return an error if m is not zero value forcing a mutual acknowledgement that m won't be use to filter.
There was a problem hiding this comment.
emm, so just one function. Combine select and selectall always receive a model parameter, when conditionalAPI Select checks the first m parameter is empty and consistent with the conditional model.
There was a problem hiding this comment.
Oh, I didn't think that the same receiver implemented both interfaces.
So, we would have on both interfaces
Select(m model.Model, fields ...any) ([]ovsdb.Operation, error) {
The behavior for the non-conditional API one could potentially check that m is a zero value.
The behavior of the conditional API one is okay as it is in terms of checks I think.
And now we can think of a proper name for a method that would just get all columns or leave that exercise for the future.
| // Extract model type from predicate function signature | ||
| predicateValue := reflect.ValueOf(predicate) | ||
| if predicateValue.Kind() != reflect.Func { | ||
| return nil, fmt.Errorf("predicate must be a function") | ||
| } | ||
|
|
||
| predicateType := predicateValue.Type() | ||
| if predicateType.NumIn() != 1 { | ||
| return nil, fmt.Errorf("predicate function must have exactly one parameter") | ||
| } | ||
|
|
||
| paramType := predicateType.In(0) | ||
| if paramType.Kind() != reflect.Ptr { | ||
| return nil, fmt.Errorf("predicate function parameter must be a pointer to a model") | ||
| } | ||
|
|
||
| // Create a zero-value instance of the model type | ||
| modelType := paramType.Elem() | ||
| modelInstance := reflect.New(modelType).Interface() | ||
|
|
||
| // Verify it implements model.Model interface | ||
| if _, ok := modelInstance.(model.Model); !ok { | ||
| return nil, fmt.Errorf("predicate function parameter must be a pointer to a model.Model") | ||
| } | ||
|
|
There was a problem hiding this comment.
Does this work? I don't think the fields provided would be a pointer to the fields of the model obtained here?
At this point I think it would be simpler to just have the same as API as when non conditional:
WhereXXX(...).Select(model, fields)
If we are really bothered that model has to potentially be passed twice in the Where and Select, we can split it to two APIs:
WhereXXX(...).Select() // all fields
WhereXXX(...).SelectFields(model, fields)
halfcrazy
force-pushed
the
select-refact
branch
2 times, most recently
from
2d33bbf to
3eb6a6e
Compare
jcaamano
left a comment
jcaamano
left a comment
There was a problem hiding this comment.
Just a couple of nits. I think we are pretty much there so feel free to squash.
@dave-tucker would please take a second look to the API in case you see something bad we have missed?
| ovsdbConditionsList, err = a.cond.Generate() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to generate conditions for select: %w", err) | ||
| } |
There was a problem hiding this comment.
nit
In other instances when Generate fails, we just return the Generate error directly. I would do the same here.
| // GetSelectResultsByIndex parses the results of a transaction containing select operations | ||
| // and populates the target slice with the specified select query's results. | ||
| // The index parameter specifies which select query to retrieve (0-based). | ||
| // Use index=0 for single select queries (WhereAny, WhereCache, etc.). | ||
| GetSelectResultsByIndex(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}, index int) error | ||
| GetSelectResults(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}) error |
There was a problem hiding this comment.
nit so that IDEs present better doc for the methods
| // GetSelectResultsByIndex parses the results of a transaction containing select operations | |
| // and populates the target slice with the specified select query's results. | |
| // The index parameter specifies which select query to retrieve (0-based). | |
| // Use index=0 for single select queries (WhereAny, WhereCache, etc.). | |
| GetSelectResultsByIndex(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}, index int) error | |
| GetSelectResults(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}) error | |
| // GetSelectResultsByIndex parses the result of the select operation indicated by | |
| // the 0-based index from the transaction results of the provided operations. | |
| GetSelectResultsByIndex(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}, index int) error | |
| // GetSelectResults parses the result of the first select operation from the | |
| // transaction results of the provided operations. | |
| GetSelectResults(ops []ovsdb.Operation, results []ovsdb.OperationResult, target interface{}) error |
Signed-off-by: Yan Zhu <hackzhuyan@gmail.com>
halfcrazy
force-pushed
the
select-refact
branch
from
3eb6a6e to
34d7484
Compare
|
Thrilled that we’re finally at the merge stage for this PR! I know this one took a few rounds of review, but I genuinely valued every comment you shared – it not only made the code more robust but also taught me a lot about api design. Thanks for taking the time to walk through this with me step by step. Couldn’t have gotten here without your guidance! |
@halfcrazy thanks for the patience and effort you have put here, this is a feature we had been looking forward to. |
3 participants
Fix #128 #301
For details, see the readme.