metabase: Check parameter for CountAliveObjectsInBucket #1414

acid-ant · 2024-10-04T12:27:25Z

acid-ant commented

2024-10-04 12:27:25 +00:00

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>

acid-ant force-pushed bugfix/count-in-bucket from d0f1c6d37b to b96d5c9b02

2024-10-04 12:28:04 +00:00

Compare

acid-ant requested review from storage-core-committers 2024-10-04 12:28:24 +00:00

acid-ant requested review from storage-core-developers 2024-10-04 12:28:30 +00:00

dstepanov-yadro approved these changes 2024-10-04 12:32:18 +00:00

Dismissed

fyrchik approved these changes 2024-10-04 12:34:51 +00:00

Dismissed

pkg/local_object_storage/metabase/list.go Outdated

					
				@ -454,3 +454,2 @@

					cidRaw := prm.BucketName[1:bucketKeySize]

					if cidRaw == nil {

					if prm.BucketName == nil {

fyrchik commented

2024-10-04 12:34:48 +00:00

if len(prm.BucketName) == 0 would be nicer, it prevents a possible panic which is not caught by nil comparison

`if len(prm.BucketName) == 0` would be nicer, it prevents a possible panic which is not caught by `nil` comparison

acid-ant commented

2024-10-04 12:48:08 +00:00

Thanks, fixed.

fyrchik commented

2024-10-04 12:51:40 +00:00

BTW, it still can be less than the bucketKeySize
Not for this PR: is there any reason we leak BucketName to the upper level?
It seems enough to operate with some notion of object type and container id, both of which are already existing abstractions.

BTW, it still can be less than the `bucketKeySize` Not for this PR: is there any reason we leak `BucketName` to the upper level? It seems enough to operate with some notion of `object type` and `container id`, both of which are already existing abstractions.

acid-ant commented

2024-10-04 13:42:01 +00:00

Right, need to check for bucketKeySize. Updated.
As for BucketName - no strong reason, just for quicker implementation and less compute.
I'll update method signature - will use object type and container id.

Right, need to check for `bucketKeySize`. Updated. As for `BucketName` - no strong reason, just for quicker implementation and less compute. I'll update method signature - will use object type and container id.