pnowojski commented on code in PR #29624:
URL: https://github.com/apache/flink/pull/29624#discussion_r4400796086
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/common/CommonExecValues.java:
##########
@@ -69,16 +71,36 @@ protected Transformation<RowData> translateToPlanInternal(
(RowType) getOutputType(),
tuples,
getClass().getSimpleName());
- final Transformation<RowData> transformation =
- planner.getExecEnv()
- .createInput(inputFormat,
inputFormat.getProducedType())
- .getTransformation();
+ final Transformation<RowData> transformation;
+ // Without rows, VALUES keeps the legacy source: a DataGeneratorSource
emits at least one.
+ if (legacySourceEnabled() || tuples.isEmpty()) {
Review Comment:
> If you want, I can make DataGeneratorSource support a count of 0 in a
separate PR."
If anything, this should be part of this PR as one of the first commits, so
that we don't have to modify this code here twice.
I think it would be slightly cleaner to support empty sequences in
`NumberSequenceSource` - by adding a support for `to == -1` and
`checkState(from <= to || to == -1)`. But I think that's worth only if such
change is trivial, like only a couple of lines of code. Otherwise one could
aruge that keeping legacy source for this case might be better?
--
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]
