Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abstract IncrementalIndex cursor stuff to prepare for using different "views" of the data based on the cursor build spec #17064

Conversation

clintropolis
Copy link
Member

changes:

  • introduce IncrementalIndexRowSelector interface to capture how IncrementalIndexCursor and IncrementalIndexColumnSelectorFactory read data
  • IncrementalIndex implements IncrementalIndexRowSelector
  • move FactsHolder interface to separate file
  • other minor refactorings

…y of using different "views" of the data per the cursor build spec

changes:
* introduce `IncrementalIndexRowSelector` interface to capture how `IncrementalIndexCursor` and `IncrementalIndexColumnSelectorFactory` read data
* `IncrementalIndex` implements `IncrementalIndexRowSelector`
* move `FactsHolder` interface to separate file
* other minor refactorings
@clintropolis clintropolis changed the title abstract IncrementalIndex cursor to prepare to allow for possibility of using different "views" of the data per the cursor build spec abstract IncrementalIndex cursor stuff to prepare to allow for possibility of using different "views" of the data per the cursor build spec Sep 14, 2024
@clintropolis clintropolis changed the title abstract IncrementalIndex cursor stuff to prepare to allow for possibility of using different "views" of the data per the cursor build spec abstract IncrementalIndex cursor stuff to prepare for using different "views" of the data per the cursor build spec Sep 14, 2024
@clintropolis clintropolis changed the title abstract IncrementalIndex cursor stuff to prepare for using different "views" of the data per the cursor build spec abstract IncrementalIndex cursor stuff to prepare for using different "views" of the data based on the cursor build spec Sep 14, 2024
this.metricIndex = metricIndex;
this.rowSelector = rowSelector;
this.metricIndex = metricDesc.getIndex();
this.classOfObject = ComplexMetrics.getSerdeForType(metricDesc.getType()).getObjectStrategy().getClazz();

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
ComplexMetricSerde.getObjectStrategy
should be avoided because it has been deprecated.
/**
* Position of {@link org.apache.druid.segment.column.ColumnHolder#TIME_COLUMN_NAME} in the dimensions list
*/
int getTimePosition();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a weird method, because the dimension order isn't provided by other methods, so it isn't meaningful on its face. The only caller is just checking if it == 0, so it can populate a List<OrderBy>.

How about replacing this with List<OrderBy> getOrdering()?

@clintropolis clintropolis merged commit 73a6442 into apache:master Sep 15, 2024
90 checks passed
@clintropolis clintropolis deleted the incremental-index-row-selector-abstraction branch September 15, 2024 23:45
pranavbhole pushed a commit to pranavbhole/druid that referenced this pull request Sep 17, 2024
…nt "views" of the data based on the cursor build spec (apache#17064)

* abstract `IncrementalIndex` cursor stuff to prepare to allow for possibility of using different "views" of the data based on the cursor build spec
changes:
* introduce `IncrementalIndexRowSelector` interface to capture how `IncrementalIndexCursor` and `IncrementalIndexColumnSelectorFactory` read data
* `IncrementalIndex` implements `IncrementalIndexRowSelector`
* move `FactsHolder` interface to separate file
* other minor refactorings
clintropolis added a commit to clintropolis/druid that referenced this pull request Oct 5, 2024
…nt "views" of the data based on the cursor build spec (apache#17064)

* abstract `IncrementalIndex` cursor stuff to prepare to allow for possibility of using different "views" of the data based on the cursor build spec
changes:
* introduce `IncrementalIndexRowSelector` interface to capture how `IncrementalIndexCursor` and `IncrementalIndexColumnSelectorFactory` read data
* `IncrementalIndex` implements `IncrementalIndexRowSelector`
* move `FactsHolder` interface to separate file
* other minor refactorings
abhishekagarwal87 pushed a commit that referenced this pull request Oct 5, 2024
* abstract `IncrementalIndex` cursor stuff to prepare for using different "views" of the data based on the cursor build spec (#17064)

* abstract `IncrementalIndex` cursor stuff to prepare to allow for possibility of using different "views" of the data based on the cursor build spec
changes:
* introduce `IncrementalIndexRowSelector` interface to capture how `IncrementalIndexCursor` and `IncrementalIndexColumnSelectorFactory` read data
* `IncrementalIndex` implements `IncrementalIndexRowSelector`
* move `FactsHolder` interface to separate file
* other minor refactorings

* add DataSchema.Builder to tidy stuff up a bit (#17065)

* add DataSchema.Builder to tidy stuff up a bit

* fixes

* fixes

* more style fixes

* review stuff

* Projections prototype (#17214)
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 10, 2024
@kfaraz
Copy link
Contributor

kfaraz commented Oct 10, 2024

This has already been backported in #17257 , so added it to the milestone.

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.

3 participants