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

Add PreAgg Hint #1617

Merged
merged 2 commits into from Sep 6, 2019
Merged

Add PreAgg Hint #1617

merged 2 commits into from Sep 6, 2019

Conversation

worker24h
Copy link
Contributor

We can force open PreAgg function if it has'nt aggregation information in SQL.

@@ -338,6 +338,7 @@ nonterminal ArrayList<String> opt_plan_hints;
nonterminal ArrayList<String> opt_sort_hints;
nonterminal Expr sign_chain_expr;
nonterminal Qualifier union_op;
nonterminal String opt_agg_open_hints;
Copy link
Contributor

Choose a reason for hiding this comment

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

change this name? This can be hint for other purpose.
I think it should be a List, cause we can support other hints later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@@ -70,6 +70,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
isAnalyzed = true; // true that we have assigned desc
analyzeJoin(analyzer);
analyzeSortHints();
analyzeAggOpenedHints();
Copy link
Contributor

Choose a reason for hiding this comment

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

analyzeHints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

((OlapScanNode) root).setIsPreAggregation(false, turnOffReason);
if ((root instanceof OlapScanNode)) {
OlapScanNode node = (OlapScanNode) root;
for (TableRef tableRef : selectStmt.getTableRefs()) {// Force Open PreAgg
Copy link
Contributor

Choose a reason for hiding this comment

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

what do this logic means to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We must have to match the table name,so we can set OpenPreAgg flags

@@ -327,6 +325,10 @@ private void turnOffPreAgg(AggregateInfo aggInfo, SelectStmt selectStmt, Analyze
break;
}

if (((OlapScanNode)root).getForceOpenPreAgg()) {
return;//OlapScanNode has be forced opened PreAgg then the function return directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

why return?
you should call olapNode.setIsPreAggregation(true, null);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if return value of the function getForceOpenPreAgg() is true, the OlapScanNode has be set TRUE,so we don't call olapNode.setIsPreAggregation(true, null);

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

FLit1 = [0-9]+ \. [0-9]*
FLit2 = \. [0-9]+
FLit3 = [0-9]+
Exponent = [eE] [+-]? [0-9]+
DoubleLiteral = ({FLit1}|{FLit2}|{FLit3}) {Exponent}?

EolHintBegin = "--" " "* "+"
CommentedHintBegin = "/*" " "* "+"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think your change will cause problem for query like "select /* test */ a from test"

Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit 2f52ae7 into apache:master Sep 6, 2019
@imay imay mentioned this pull request Sep 26, 2019
@worker24h worker24h deleted the preagg-hint branch February 17, 2020 08:21
luwei16 pushed a commit to luwei16/incubator-doris that referenced this pull request Apr 7, 2023
SWJTU-ZhangLei pushed a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants