MartijnVisser commented on code in PR #29566:
URL: https://github.com/apache/flink/pull/29566#discussion_r4359101120
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/ptf/BuiltInProcessTableFunction.java:
##########
@@ -76,7 +99,85 @@ protected BuiltInProcessTableFunction(
.orElseThrow(IllegalStateException::new);
this.internalStateStrategies =
- toInternalStateStrategies(definitionTypeInference,
callContext);
+ internalStateStrategies != null
+ ? internalStateStrategies
+ : toInternalStateStrategies(definitionTypeInference,
callContext);
Review Comment:
Nothing here calls `internalState` or `customState`, and #29365 adds its own
`toInternalStateType`. Will #29365 switch to these helpers?
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/ptf/BuiltInProcessTableFunction.java:
##########
@@ -76,7 +99,85 @@ protected BuiltInProcessTableFunction(
.orElseThrow(IllegalStateException::new);
this.internalStateStrategies =
- toInternalStateStrategies(definitionTypeInference,
callContext);
+ internalStateStrategies != null
+ ? internalStateStrategies
+ : toInternalStateStrategies(definitionTypeInference,
callContext);
+ }
+
+ /**
+ * Declares the state entries of a specialized function in order. The
strategies may infer
+ * external data types, they are converted to internal ones.
+ */
+ @SafeVarargs
+ protected static LinkedHashMap<String, StateTypeStrategy> internalState(
+ final Map.Entry<String, StateTypeStrategy>... entries) {
+ return state(BuiltInProcessTableFunction::toInternalStateStrategy,
entries);
+ }
+
+ /**
+ * Declares the state entries of a specialized function in order. The
strategies must already
+ * infer the data types that the function works on, no internal conversion
is applied.
+ */
+ @SafeVarargs
+ protected static LinkedHashMap<String, StateTypeStrategy> customState(
+ final Map.Entry<String, StateTypeStrategy>... entries) {
+ return state(UnaryOperator.identity(), entries);
+ }
+
+ private static LinkedHashMap<String, StateTypeStrategy> state(
+ final UnaryOperator<StateTypeStrategy> mapper,
+ final Map.Entry<String, StateTypeStrategy>[] entries) {
+ final LinkedHashMap<String, StateTypeStrategy> result = new
LinkedHashMap<>();
+ for (final Map.Entry<String, StateTypeStrategy> entry : entries) {
+ result.put(entry.getKey(), mapper.apply(entry.getValue()));
+ }
+ return result;
+ }
+
+ private static StateTypeStrategy toInternalStateStrategy(final
StateTypeStrategy strategy) {
Review Comment:
I think `toInternalStateStrategies` could use this too, so
definition-declared state keeps its view wrapper and `isBroadcast`. PTF suites
stay green with that locally.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/StateTypeStrategy.java:
##########
@@ -19,13 +19,25 @@
package org.apache.flink.table.types.inference;
import org.apache.flink.annotation.PublicEvolving;
+import org.apache.flink.table.functions.ProcessTableFunction;
+import org.apache.flink.table.types.DataType;
import javax.annotation.Nullable;
import java.time.Duration;
import java.util.Optional;
-/** Strategy for inferring a function call's intermediate result data type
(i.e. state entry). */
+/**
+ * Strategy for inferring a function call's intermediate result data type
(i.e. state entry).
+ *
+ * <p>For {@link ProcessTableFunction}, returning {@link Optional#empty()}
from {@link
Review Comment:
Fair enough. Users can rely on it now though, so the Jira should get a
release note.
--
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]
