-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[improvement](meta) Infer the column name when create view if the column is expression #24990
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
[improvement](meta) Infer the column name when create view if the column is expression #24990
Conversation
…umn is expression
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
|
||
| @Override | ||
| protected String getExprName() { | ||
| return Expression.normalizeColumnName(getFnCall().getExprName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function should in org.apache.doris.catalog.Column or under org.apache.doris.common package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rethink this method, I think the actual mean of the method is that normalize the name of expression or expr. and
it should be independent to column domain or other. So I think should rename it to normalizeName to match the actual meaning for normalize the expr(expression name). WDYT?
| // alias or is not slotRef | ||
| protected String getExprName() { | ||
| if (StringUtils.isEmpty(this.exprName)) { | ||
| return Expression.DEFAULT_COLUMN_NAME; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
| * Need the position of selectListItem to generate column label | ||
| */ | ||
| public String toColumnLabel(int position) { | ||
| Preconditions.checkState(!isStar()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add check msg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have fixed.
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
please replace picture with md's table in PR desc for search and git commit msg friendly |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
run buildall |
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
| * Abstract class for all Expression in Nereids. | ||
| */ | ||
| public abstract class Expression extends AbstractTreeNode<Expression> implements ExpressionTrait { | ||
| public static final String DEFAULT_EXPRESSION_NAME = "__expression"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is different from Expr.DEFAULT_EXPR_NAME?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expression is for the new optimizer, Expr is for the old representation. Maybe we need to distinguish them by different names.
| } | ||
|
|
||
| @Override | ||
| protected String getExpressionName() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems same as Expression.getExpressionName()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, this is same to Expression.getExpressionName(), it can be removed. I will fix it.
|
|
||
| @Override | ||
| protected String getExpressionName() { | ||
| return "literal"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as Expression.getExpressionName()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Litreal has some child classes such as FloatLiteral, StringLiteral and so on, I think they can be literal uniformly.
So I overwrite the getExpressionName with return "literal".
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
morningman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
…izer (#25317) Disable infer expr column name when query on old optimizer. This bug is be brought in #24990 if your query SQL is select id, name, sum(target) FROM db_test.table_test2 group by id, name; the result column name when query is as following: |id|name |sum(cast(target as DOUBLE))| when you create view as following: CREATE VIEW v1 as select id, name, sum(target) FROM db_test.table_test2 group by id, name; then query the view v1, the result is as following: |id|name |__sum_2|
…select into outfile` (#25854) This pr do two things: 1. Infer the column name if the column is expression in `select into outfile`. The rule for column name generation can be refered in pr: #24990 2. fix bug that it will core dump if the `_schema` fails to build in the open phase in vorc_transformer.cpp TODO: 1. Support infer the column name if the column is expression in `select into outfile` in new optimizer(Nereids).
…umn is expression (apache#24990) ## Proposed changes Infer the column name when create view if the column is expression ## Further comments expr column name infer strategy as following: | expr | example | column name(before) | Inferred column name(if position is 2) | | ------------- | --------------------------------------- | ------------------------------ | -------------------------------------- | | function | dayofyear() | dayofyear() | __dayofyear_1 | | cast | cast(1 as bigint) | CAST(1 AS BIGINT) | __cast_1 | | anylyticExpr | min() | min() | __min_1 | | predicate | 1 in (1,2,3,4) | 1 IN (1, 2, 3, 4) | __in_predicate_1 | | literal | 1 or 'string_var_name' | 1 or 'string_var_name' | __literal_1 | | arithmeticExpr | & | ... & ... | __arithmetic_expr_1 | | identifier | a or b | a or b | a or b | | case | CASE WHEN remark = 's' THEN 1 ELSE 2 END | CASE WHEN remark = 's' THEN 1 ELSE 2 END | __case_1 | | window | min(timestamp) OVER (...) | min(timestamp) OVER(...) | __min_1 | SQL for example: ```sql CREATE VIEW v1 AS SELECT error_code, 1, 'string', now(), dayofyear(op_time), cast (source AS BIGINT), min(`timestamp`) OVER ( ORDER BY op_time DESC ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING ), 1 > 2, 2 + 3, 1 IN (1, 2, 3, 4), remark LIKE '%like', CASE WHEN remark = 's' THEN 1 ELSE 2 END, TRUE | FALSE FROM db_test.table_test1 ``` the output column name is as following: ``` error_code __literal_1 __literal_2 __now_3 __dayofyear_4 __cast_expr_5 __min_6 __binary_predicate_7 __arithmetic_expr_8 __in_predicate_9 __like_predicate_10 __case_expr_11 __arithmetic_expr_12 ```
…izer (apache#25317) Disable infer expr column name when query on old optimizer. This bug is be brought in apache#24990 if your query SQL is select id, name, sum(target) FROM db_test.table_test2 group by id, name; the result column name when query is as following: |id|name |sum(cast(target as DOUBLE))| when you create view as following: CREATE VIEW v1 as select id, name, sum(target) FROM db_test.table_test2 group by id, name; then query the view v1, the result is as following: |id|name |__sum_2|
…select into outfile` (apache#25854) This pr do two things: 1. Infer the column name if the column is expression in `select into outfile`. The rule for column name generation can be refered in pr: apache#24990 2. fix bug that it will core dump if the `_schema` fails to build in the open phase in vorc_transformer.cpp TODO: 1. Support infer the column name if the column is expression in `select into outfile` in new optimizer(Nereids).
Infer name if it is an expression and doesn't alias artificially when create or select stmt in nereids. The infer name strategy is the same as #24990
…e#26055) Infer name if it is an expression and doesn't alias artificially when create or select stmt in nereids. The infer name strategy is the same as apache#24990
…izer (apache#25317) Disable infer expr column name when query on old optimizer. This bug is be brought in apache#24990 if your query SQL is select id, name, sum(target) FROM db_test.table_test2 group by id, name; the result column name when query is as following: |id|name |sum(cast(target as DOUBLE))| when you create view as following: CREATE VIEW v1 as select id, name, sum(target) FROM db_test.table_test2 group by id, name; then query the view v1, the result is as following: |id|name |__sum_2|
…select into outfile` (apache#25854) This pr do two things: 1. Infer the column name if the column is expression in `select into outfile`. The rule for column name generation can be refered in pr: apache#24990 2. fix bug that it will core dump if the `_schema` fails to build in the open phase in vorc_transformer.cpp TODO: 1. Support infer the column name if the column is expression in `select into outfile` in new optimizer(Nereids).
…e#26055) Infer name if it is an expression and doesn't alias artificially when create or select stmt in nereids. The infer name strategy is the same as apache#24990
Proposed changes
Infer the column name when create view if the column is expression
Further comments
expr column name infer strategy as following:
SQL for example:
the output column name is as following: