Skip to content

fix: constant columns for clustered segment#19675

Open
clintropolis wants to merge 3 commits into
apache:masterfrom
clintropolis:clustered-segment-improvements-and-fixes
Open

fix: constant columns for clustered segment#19675
clintropolis wants to merge 3 commits into
apache:masterfrom
clintropolis:clustered-segment-improvements-and-fixes

Conversation

@clintropolis

Copy link
Copy Markdown
Member

Description

changes:

  • QueryableIndex#getClusterGroupQueryableIndex now accepts a boolean flag to control whether or not constant columns are created for the clustering columns (true for queries, false for persist/merge)
  • these constants resolve some issues around filtering clustered segments

changes:
* `QueryableIndex#getClusterGroupQueryableIndex` now accepts a boolean flag to control whether or not constant columns are created for the clustering columns (true for queries, false for persist/merge)
* these constants resolve some issues around filtering clustered segments

@capistrant capistrant left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One observation regarding defensiveness for you to consider

"clustering column [%s] is missing a type",
columnName
));
final BaseColumnHolder holder = makeConstantColumnHolder(columnType, clusteringValues[i], numRows, bitmapFactory);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting trust in caller to pass a good clusteringValues array. bad callsite use could index oob. perhaps worth a defensive size comparison with clusteringColumns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants