colfetcher: use IndexFetchSpec#75767
Conversation
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, 9 of 9 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde)
pkg/sql/colfetcher/cfetcher.go, line 1140 at r2 (raw file):
// processValueSingle processes the given value (of column // family.DefaultColumnID), setting values in cf.machine.colvecs accordingly.
nit: family.DefaultColumnID is no more.
pkg/sql/colfetcher/cfetcher_setup.go, line 36 at r2 (raw file):
spec descpb.IndexFetchSpec // ColIdxMap is a mapping from ColumnID to the ordinal of the corresponding // column within the cols field.
nit: something like s/cols/spec.FetchedColumns/.
pkg/sql/colfetcher/cfetcher_setup.go, line 38 at r2 (raw file):
// column within the cols field. ColIdxMap catalog.TableColMap // typs are the types of spec.FetchColumns.
nit: I think FetchedColumns.
pkg/sql/colfetcher/cfetcher_setup.go, line 172 at r2 (raw file):
} // Remapt he post processing spec. away columns that aren't needed.
nit: s/Remapt he/Remap the/, then there is something off in "spec. away".
pkg/sql/row/errors.go, line 225 at r1 (raw file):
allColumns bool, ) (_ catalog.Index, columnNames []string, columnValues []string, _ error) { // Strip the tenant prefix and pretend to use the system tenant's SQL codec
nit: the comment needs an update.
This change refactors the `row.Fetcher` error handling code to no longer rely on the table descriptor. The interesting part is where we decode key values for a locking error; we now use the `IndexFetchSpec` and decode the key directly rather than setting up a fake fetcher through `DecodeRowInfo`. Release note: None
This commit converts the cFetcher to use an IndexFetchSpec instead of table and index descriptors. Release note: None
21b0e95 to
1d2f1df
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @yuzefovich)
|
bors r+ |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build failed (retrying...): |
|
Build succeeded: |
row: refactor error handling code
This change refactors the
row.Fetchererror handling code to nolonger rely on the table descriptor. The interesting part is where we
decode key values for a locking error; we now use the
IndexFetchSpecand decode the key directly rather than setting up a fake fetcher
through
DecodeRowInfo.Release note: None
colfetcher: use IndexFetchSpec
This commit converts the cFetcher to use an IndexFetchSpec instead of
table and index descriptors.
Release note: None