Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support UDF in planner #2995
base: main
Are you sure you want to change the base?
Support UDF in planner #2995
Changes from 70 commits
3a5a44b
53416bf
3f88bb4
773e0dd
fa472d7
4c32432
ccd8e8d
403a4a2
cada59a
3b02019
138a7fc
f8b3658
b4edc0a
c42c0ce
a570479
f4e5cac
c10ec5b
0d9cc50
057fee0
37c8641
a52d15a
74c24fb
2f33668
01c4d01
8c70b2a
5d59fa2
7e84176
0d54f1e
543a79c
8fd8208
f5b3314
9956a9f
5476bb4
f9fd6a9
651816c
2c886c8
38a8004
4f7a23f
25eb6cf
6ebd19b
f8088f5
157ab0c
0daa4ce
9a325bf
bffaca4
b3589d2
46eaaf1
523859e
18312ed
0a70d6e
d058c09
67fad06
e11b315
29b9cb2
52b3ba4
91e6117
262f03b
b8eb6f1
94bc71d
8549291
9610aa4
6ff48c0
bd1cd47
b67cbb7
2cb7ebd
c82fa36
1635552
3bda453
8ba674f
385aa1d
25aa001
32c0e42
cd3114c
67cb050
e61289b
7e3ebf9
59ec547
d7bc203
d0c881f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
to make review easier, all changes in this file is to change the 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.
Can you ask Alec to review the proto stuff as well?
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 approach you have taken is not what I had in mind:
BuiltInFunction
should give up most of its attributes/functionality, encapsulation to this class. Only the encapsulation lambda should stay that is used to implement theencapsulate()
method.BuiltInFunction
should be THE base for all internal functions that are not domain specific (likelength(string)
or similar).MacroFunction
should be extending this class and adding the functionality that you coded up to expand an arbitrary bodyThere 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.
discussed offline, restructured the code.
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.
Can this be called something that encompasses more than just serialization. I had suggested
CatalogedFunction
.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.
discussed offline, renamed.
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.
can add more
if
blocks when we have more items in theoneof
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.
will do dynamic dispatch in a follow-up PR