Core: Adjust calculations for reserved field IDs#16441
Conversation
nastra
force-pushed
the
content-stats-calculations
branch
2 times, most recently
from
adeecba to
563110b
Compare
| long statsFieldId = | ||
| STATS_SPACE_FIELD_ID_START_FOR_METADATA_FIELDS | ||
| + (NUM_SUPPORTED_STATS_PER_COLUMN * (long) offset); | ||
| if (statsFieldId < 0 || statsFieldId > RESERVED_FIELD_IDS_START) { |
There was a problem hiding this comment.
we don't need to check for the overflow anymore, because that's already guaranteed that we only ever do this calculation for the field IDs in SUPPORTED_METADATA_FIELD_IDS
| } | ||
|
|
||
| private static int fieldIdForStatsFieldFromReservedField(int statsFieldId) { | ||
| return Math.max( |
There was a problem hiding this comment.
this isn't needed anymore, because the caller already guarantees that 9000 <= statsFieldId < 1000, so this can never be negative
nastra
force-pushed
the
content-stats-calculations
branch
from
563110b to
60e24fe
Compare
| // the starting field ID of the reserved field ID space | ||
| static final int RESERVED_FIELD_IDS_START = Integer.MAX_VALUE - NUM_RESERVED_FIELD_IDS; | ||
| // the number of supported stats per table column | ||
| private static final int FIRST_SUPPORTED_METADATA_FIELD_ID = |
There was a problem hiding this comment.
To avoid future code drift, consider moving SUPPORTED_METADATA_FIELD_IDS ahead, and then FIRST_SUPPORTED_METADATA_FIELD_ID would just be Collections.min(SUPPORTED_METADATA_FIELD_IDS)
| private static final int FIRST_SUPPORTED_METADATA_FIELD_ID = | ||
| MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.fieldId(); | ||
| static final Set<Integer> SUPPORTED_METADATA_FIELD_IDS = | ||
| Sets.newHashSet( |
| } | ||
|
|
||
| @Test | ||
| public void statsIdsForReservedColumns() { |
| MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.fieldId(), MetadataColumns.ROW_ID.fieldId()); | ||
| static final int NUM_SUPPORTED_STATS_PER_COLUMN = 200; | ||
| // the starting field ID of the stats space for data field IDs | ||
| static final int STATS_SPACE_FIELD_ID_START_FOR_METADATA_FIELDS = 9_000; |
There was a problem hiding this comment.
Should we do a Precondition check in a static initializer block to make sure SUPPORTED_METADATA_FIELD_IDS in future don't exceed 5 (the number of slots available between 9K and 10K)?
2 participants
This adjusts the calculations around stats field IDs for data and metadata columns and also updates the code to only support stats for the stats ID range
[10_000, 200_000_000).The PR was created with the help of Claude Opus 4.7 and manually adjusted in a few places.