nateab commented on code in PR #28114:
URL: https://github.com/apache/flink/pull/28114#discussion_r3283615268
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/DescribeFunctionOperation.java:
##########
@@ -114,19 +121,78 @@ public TableResultInternal execute(Context ctx) {
Arrays.asList(
"supports constant folding",
String.valueOf(definition.supportsConstantFolding())));
+ final TypeInference typeInference =
+
definition.getTypeInference(ctx.getCatalogManager().getDataTypeFactory());
rows.add(
Arrays.asList(
"signature",
- generateSignature(
- definition.getTypeInference(
-
ctx.getCatalogManager().getDataTypeFactory()),
- function.toString(),
- definition)));
+ generateSignature(typeInference,
function.toString(), definition)));
+ rows.addAll(buildPtfMetadataRows(definition, typeInference));
}
return buildTableResult(
new String[] {"info name", "info value"},
new DataType[] {DataTypes.STRING(), DataTypes.STRING()},
rows.stream().map(List::toArray).toArray(Object[][]::new));
}
+
+ /**
+ * Builds supplemental info rows from the given {@link FunctionDefinition}
and {@link
+ * TypeInference}: a {@code state: <name>} row per state entry (with type
and TTL) and, for
+ * PROCESS_TABLE functions, three capability rows: {@code accepts system
arguments} (whether the
+ * framework auto-injects {@code uid} / {@code on_time}), {@code is
changelog function} (whether
+ * the function implements {@link ChangelogFunction}), and {@code uses
timers} (whether the
+ * function declares an {@code onTimer} method). Returns an empty list
when none apply.
+ */
+ private static List<List<Object>> buildPtfMetadataRows(
+ FunctionDefinition definition, TypeInference typeInference) {
+ final List<List<Object>> rows = new ArrayList<>();
+ typeInference
+ .getStateTypeStrategies()
+ .forEach(
+ (name, strategy) ->
+ rows.add(
+ Arrays.asList(
+ "state: " + name,
formatStateEntry(strategy))));
+ if (definition.getKind() == FunctionKind.PROCESS_TABLE) {
+ rows.add(
+ Arrays.asList(
+ "accepts system arguments",
+
String.valueOf(!typeInference.disableSystemArguments())));
+ rows.add(
+ Arrays.asList(
+ "is changelog function",
+ String.valueOf(definition instanceof
ChangelogFunction)));
+ rows.add(
+ Arrays.asList(
+ "uses timers",
+ String.valueOf(
+ !ExtractionUtils.collectMethods(
+ definition.getClass(),
+ UserDefinedFunctionHelper
+
.PROCESS_TABLE_ON_TIMER)
+ .isEmpty())));
+ }
+ return rows;
+ }
+
+ private static String formatStateEntry(StateTypeStrategy strategy) {
+ // We have no CallContext at DESCRIBE time. Many strategies (including
TTL lookups in
+ // DefaultStateTypeStrategy) ignore the context, but inferType
forwards it to the wrapped
+ // TypeStrategy which may dereference it. Catch and degrade to
<unknown> rather than
+ // failing the entire DESCRIBE for one strategy that needs a real
CallContext.
+ String typeStr;
+ try {
+ typeStr =
strategy.inferType(null).map(Object::toString).orElse("<unknown>");
+ } catch (Exception e) {
+ typeStr = "<unknown>";
+ }
+ final Optional<Duration> ttl;
+ try {
+ ttl = strategy.getTimeToLive(null);
+ } catch (Exception e) {
Review Comment:
<p>Good catch, addressed in <code>2e24d12cf6e</code>.I went with a hybrid
of both options:</p>
<ul>
<li><strong>(a)</strong> Added javadoc to
<code>StateTypeStrategy#getTimeToLive</code> and
<code>TypeStrategy#inferType</code> noting that <code>DESCRIBE FUNCTION
EXTENDED</code> may pass <code>null</code> and that implementations should
handle it defensively (return <code>Optional.empty()</code>); throwing renders
the value as <code><unknown></code>.</li>
<li><strong>(b)</strong> Added a static SLF4J logger to
<code>DescribeFunctionOperation</code> and <code>LOG.debug(...)</code> the
swallowed exception in both catch blocks, so the cause is diagnosable.</li>
</ul>
<p>Kept the catch at <code>Exception</code> rather than narrowing to
<code>NullPointerException</code>, a strategy that defensively throws (e.g.
<code>IllegalStateException("CallContext is required")</code>) shouldn't fail
the entire <code>DESCRIBE</code> for one function, and that wouldn't be an NPE.
The safety
net should be against any introspection failure, not just null deref.</p>
--
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]
