Skip to content

Fix summary row issues in case postaggregations are happening#15232

Merged
soumyava merged 11 commits intoapache:masterfrom
kgyrtkirk:windowing-error-hint
Oct 25, 2023
Merged

Fix summary row issues in case postaggregations are happening#15232
soumyava merged 11 commits intoapache:masterfrom
kgyrtkirk:windowing-error-hint

Conversation

@kgyrtkirk
Copy link
Copy Markdown
Member

@kgyrtkirk kgyrtkirk commented Oct 23, 2023

In case the table has no negative values for m1; the following query could produce such an exception:

select count(distinct m1)*2 from druid.foo where m1 < -1.0
  • add suggerstion to enable windowing if the query uses that feature

@cryptoe cryptoe added this to the 28.0 milestone Oct 23, 2023
for (int i = 0; i < aggSpec.size(); i++) {
values[i] = aggSpec.get(i).factorize(new AllNullColumnSelectorFactory()).get();
resultRow.set(i, aggSpec.get(i).factorize(new AllNullColumnSelectorFactory()).get());
}
Copy link
Copy Markdown
Member

@clintropolis clintropolis Oct 23, 2023

Choose a reason for hiding this comment

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

oh, I think you need to actually compute the postaggs... which is why the test is failing in default value mode, see mergeResults for an example, something like

  private static Iterator<ResultRow> summaryRowIterator(GroupByQuery q)
  {
    List<AggregatorFactory> aggSpec = q.getAggregatorSpecs();
    ResultRow resultRow = ResultRow.create(q.getResultRowSizeWithPostAggregators());
    for (int i = 0; i < aggSpec.size(); i++) {
      resultRow.set(i, aggSpec.get(i).factorize(new AllNullColumnSelectorFactory()).get());
    }
    Map<String, Object> map = resultRow.toMap(q);
    for (int i = 0; i < q.getPostAggregatorSpecs().size(); i++) {
      final PostAggregator postAggregator = q.getPostAggregatorSpecs().get(i);
      final Object value = postAggregator.compute(map);

      resultRow.set(q.getResultRowPostAggregatorStart() + i, value);
      map.put(postAggregator.getName(), value);
    }
    return Collections.singleton(resultRow).iterator();
  }

makes the test pass in both modes for me

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thank you @clintropolis for taking a look and for the suggested fix :)

@github-actions github-actions Bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Oct 24, 2023
@soumyava soumyava merged commit 6784e9c into apache:master Oct 25, 2023
LakshSingla added a commit to LakshSingla/druid that referenced this pull request Oct 25, 2023
CaseyPan pushed a commit to CaseyPan/druid that referenced this pull request Nov 17, 2023
…#15232)

* fix-1/2

* add message v1

* extend test to cover for IOB issue

* move stuff around

* change message

* fix testcase string

* compute postaggs (thank you Clint!)

* enable feature for test

* ignore tests in msq

---------

Co-authored-by: Soumyava Das <soumyava@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Area - Querying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants