sql: add catalog.Partitioning interface #64013
Conversation
|
This change is |
postamar
force-pushed
the
virtualize-partitioning
branch
2 times, most recently
from
a9e817f
to
00edda4
Compare
|
i'm happy to take a look, but may i request these changes be merged after the v21.1 release is finalised (i.e. out as GA)? we may have some backports coming that will not backport nicely when these are in |
|
Yes absolutely, none of this is urgent. |
postamar
force-pushed
the
virtualize-partitioning
branch
from
00edda4
to
b0511b5
Compare
postamar
force-pushed
the
virtualize-partitioning
branch
from
b0511b5
to
2415fff
Compare
There was a problem hiding this comment.
Reviewed 27 of 27 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @postamar)
pkg/ccl/partitionccl/partition.go, line 609 at r1 (raw file):
if part.NumRange() > 0 { log.Fatal(context.TODO(), "TODO(dan): unsupported for range partitionings")
evalCtx.Context
pkg/sql/partition_utils.go, line 242 at r1 (raw file):
} if part.NumRange() > 0 {
nit: should this be called NumRanges()
pkg/sql/catalog/table_elements.go, line 407 at r1 (raw file):
// NumList returns the number of list elements in the underlying partitioning // descriptor. NumList() int
nit: NumLists() ?
postamar
force-pushed
the
virtualize-partitioning
branch
from
2415fff
to
696e9e4
Compare
There was a problem hiding this comment.
Thanks for the review!
Reviewable status:
pkg/ccl/partitionccl/partition.go, line 609 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
evalCtx.Context
Done.
pkg/sql/partition_utils.go, line 242 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: should this be called NumRanges()
Done.
pkg/sql/catalog/table_elements.go, line 407 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: NumLists() ?
Done.
There was a problem hiding this comment.
Reviewed 12 of 12 files at r2.
Reviewable status:
This commit aims at reducing direct usage of descpb.PartitioningDescriptor types and instead using the new catalog.Partitioning interface. This refactoring is in line with recent virtualization work tracked under cockroachdb#56306. In an effort to improve readability and ability to reason about state changes, this commit changes the type signature of sql.CreatePartitioning and, notably, removes its side-effects on the index descriptor. The latter is now modified using a new tabledesc.UpdateIndexPartitioning function. Release note: None
postamar
force-pushed
the
virtualize-partitioning
branch
from
696e9e4
to
a24d6c2
Compare
|
Rebased on fresh master. |
|
bors r+ |
|
Build succeeded: |
This commit aims at reducing direct usage of
descpb.PartitioningDescriptor types and instead using the new
catalog.Partitioning interface. This refactoring is in line with recent
virtualization work tracked under #56306.
In an effort to improve readability and ability to reason about state
changes, this commit changes the type signature of sql.CreatePartitioning
and, notably, removes its side-effects on the index descriptor. The
latter is now modified using a new tabledesc.UpdateIndexPartitioning
function.
Release note: None