gustavodemorais commented on code in PR #27901:
URL: https://github.com/apache/flink/pull/27901#discussion_r3046306729


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinition.java:
##########
@@ -84,7 +86,8 @@ private BuiltInFunctionDefinition(
             boolean isDeterministic,
             boolean isRuntimeProvided,
             String runtimeClass,
-            boolean isInternal) {
+            boolean isInternal,
+            @Nullable ChangelogFunction changelogFunction) {

Review Comment:
   We need to add a way for BultInFunctions to set the ChangelogMode but this 
adding it like this doesn't feel very clean. We also have to add some dead code 
below in the func definition when creating 'new ChangelogFunction()' and it's 
very specific.
   
   A simple alternative would be just going adding changelogMode to the 
BuiltInFunctionDefinition, but this might not be enough for eventual 
optimizations where we want to decide between append/upsert/retract stream 
output depending on the args inside opMapping
   
   ```
    BuiltInFunctionDefinition.newBuilder()
             .name("FROM_CHANGELOG")
             .outputChangelogMode(ChangelogMode.upsert(false))
             .build();
   ````
   
   I think we have to sync with Timo on this. Do you have other alternatives 
you think could be better to solve this? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to