Skip to content

[feature](function) support variadic template type in SQL function - #17985

Merged
xy720 merged 5 commits into
apache:masterfrom
xy720:variadic-method
Mar 28, 2023
Merged

[feature](function) support variadic template type in SQL function#17985
xy720 merged 5 commits into
apache:masterfrom
xy720:variadic-method

Conversation

@xy720

@xy720xy720 commented Mar 21, 2023

Copy link
Copy Markdown
Member

Proposed changes

In pr #17344, we already support template type in SQL function.
e.g.

[['map_keys'], 'ARRAY<K>', ['MAP<K, V>'], '', ['K', 'V']]

Then it raises a new issue, as some functions require variable length template types.
e.g.

[['struct'], 'STRUCT<T1, T2, T3, T4....>', ['T1', 'T2', 'T3', 'T4', '....'], '', ['T1', 'T2', 'T3', 'T4', '....']]

Inspired by c++ function std::vector::emplace_back(), we can use variadic template for this issue.

e.g.

[['struct'], 'STRUCT<TYPES>', ['TYPES'], 'ALWAYS_NOT_NULLABLE', ['TYPES...']]

...TYPES in template_types defines a variadic template TYPE. Then the variadic template will be expanded to multiple normal templates based on actual input arguments at runtime in FE.

But make sure TYPES... is placed on the last position in all template type arguments.

BTW, the origin template function logic is not affected.

Problem summary

Describe your changes.

Checklist(Required)

  • Does it affect the original behavior
  • Has unit tests been added
  • Has document been added or modified
  • Does it need to update dependencies
  • Is this PR support rollback (If NO, please explain WHY)

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actionsgithub-actionsBot added the area/sql/function Issues or PRs related to the SQL functions label Mar 21, 2023
@xy720

Copy link
Copy Markdown
MemberAuthor

run buildall

@hello-stephen

hello-stephen commented Mar 21, 2023

Copy link
Copy Markdown
Contributor

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.64 seconds
stream load tsv: 473 seconds loaded 74807831229 Bytes, about 150 MB/s
stream load json: 25 seconds loaded 2358488459 Bytes, about 89 MB/s
stream load orc: 76 seconds loaded 1101869774 Bytes, about 13 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230324143135_clickbench_pr_120308.html

@xy720

Copy link
Copy Markdown
MemberAuthor

run buildall

@xy720

Copy link
Copy Markdown
MemberAuthor

run buildall

@cambyzjucambyzju 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.

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Mar 24, 2023
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@xy720

Copy link
Copy Markdown
MemberAuthor

run p0

@xy720
xy720 merged commit daeaa91 into apache:masterMar 28, 2023
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 21, 2023
…pache#17985)
Inspired by c++ function `std::vector::emplace_back()`, we can use variadic template for this issue.
e.g.
```
[['struct'], 'STRUCT<TYPES>', ['TYPES'], 'ALWAYS_NOT_NULLABLE', ['TYPES...']]
```
`...TYPES` in template_types defines a variadic template `TYPE`. Then the variadic template will be expanded to multiple normal templates based on actual input arguments at runtime in FE.
But make sure `TYPES...` is placed on the last position in all template type arguments.
BTW, the origin template function logic is not affected.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.area/sql/functionIssues or PRs related to the SQL functionsreviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@xy720@hello-stephen@cambyzju