Exclude dimensions from star-tree index stored type check #13355
Conversation
| @@ -67,17 +66,11 @@ public class StarTreeIndexReader implements Closeable { | |||
| */ | |||
| public StarTreeIndexReader(File segmentDirectory, SegmentMetadataImpl segmentMetadata, ReadMode readMode) | |||
| throws IOException, ConfigurationException { | |||
| Preconditions.checkNotNull(segmentDirectory); | |||
There was a problem hiding this comment.
what's the reason for removing all these? It's guaranteed elsewhere?
There was a problem hiding this comment.
Yes. It is internal class, thus we can control all the caller behavior. We usually annotate argument that can be null with @Nullable, and skip the null check for performance reason
| @@ -268,7 +267,7 @@ private synchronized void loadData() | |||
| default: | |||
| break; | |||
| } | |||
| if (CollectionUtils.isNotEmpty(_segmentMetadata.getStarTreeV2MetadataList())) { | |||
| if (_segmentMetadata.getStarTreeV2MetadataList() != null) { | |||
There was a problem hiding this comment.
looks like you've updated it to just handle an empty list instead?
There was a problem hiding this comment.
It cannot be empty, only null or non-empty. I modified this place to make it consistent with other callers
| null, | ||
| 10)); | ||
| private final StarTreeV2BuilderConfig BUILDER_CONFIG = StarTreeV2BuilderConfig.fromIndexConfig( | ||
| new StarTreeIndexConfig(List.of("AirlineID", "Origin", "Dest"), List.of(), List.of("count__*", "max__ArrDelay"), |
There was a problem hiding this comment.
should one of the dimensions specifically have the <agg>__<name> format?
There was a problem hiding this comment.
Good suggestion. Updated BaseStarTreeV2Test to capture that
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13355 +/- ##
============================================
+ Coverage 61.75% 62.09% +0.34%
+ Complexity 207 198 -9
============================================
Files 2436 2547 +111
Lines 133233 139941 +6708
Branches 20636 21724 +1088
============================================
+ Hits 82274 86895 +4621
- Misses 44911 46450 +1539
- Partials 6048 6596 +548
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Jackie-Jiang
force-pushed
the
star_tree_stored_type
branch
from
e4f6219
to
4fe1b04
Compare
Jackie-Jiang
force-pushed
the
star_tree_stored_type
branch
from
4fe1b04
to
3af6bf0
Compare
Jackie-Jiang
force-pushed
the
star_tree_stored_type
branch
from
3af6bf0
to
c1a475f
Compare
Related to #12164 and #12554
Do not check stored type for dimensions to avoid mismatching columns with '__' in column name