Indexing: remove subspaceProvider and subspace arguments to iterating functions #3611
Conversation
… functions These two parameters, that are used for logging only, can be determined from common at the relevant logging functions. This resolves FoundationDB#3610.
…ch now contains subspace)
| if (subspaceProvider != null) { | ||
| logIf(true, keyValues, | ||
| subspaceProvider.logKey(), subspaceProvider); | ||
| } |
There was a problem hiding this comment.
Why use logIf here? It seems like you would either want to do:
| if (subspaceProvider != null) { | |
| logIf(true, keyValues, | |
| subspaceProvider.logKey(), subspaceProvider); | |
| } | |
| logIf(subspaceProvider != null, keyValues, | |
| subspaceProvider.logKey(), subspaceProvider); |
Or:
| if (subspaceProvider != null) { | |
| logIf(true, keyValues, | |
| subspaceProvider.logKey(), subspaceProvider); | |
| } | |
| if (subspaceProvider != null) { | |
| keyValues.add(subspaceProvider.logKey()); | |
| keyValues.add(subspaceProvider); | |
| } |
There was a problem hiding this comment.
The logIf was just for uniformity. The first suggestion may cause a null pointer exception as the subspaceProvider.logKey() part will be evaluated before calling the logIf function. I'll go with the second.
| .thenCompose( store -> context.getReadVersionAsync() | ||
| .thenCompose(vignore -> { | ||
| SubspaceProvider subspaceProvider = common.getRecordStoreBuilder().getSubspaceProvider(); | ||
| // validation checks, if any, will be performed here |
There was a problem hiding this comment.
Was this comment intended to be that:
buildMultiTargetIndex will do any necessary validation checks
Or, the more problematic for this change:
getSubspaceAsync will do any necessary validation checks
? I'm pretty sure the answer is that buildMultiTargetIndex is doing the validation checks, but wanted to confirm.
There was a problem hiding this comment.
This comment appeared in the indexers as a hint/reminder where to add validations. I think that appears because IndexingByIndex had a source/target indexes validations at the post context.getReadVersionAsync lambda, and then its structure was copied by the other indexers. I did not think that this comment is needed anymore.
These two parameters, that are used for logging only, can be determined from common at the relevant logging functions.
This resolves #3610.