[FLINK-19272][table-common] Add metadata interfaces for FLIP-107 #13425
Conversation
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 8e458c0 (Fri Sep 18 13:03:06 UTC 2020) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
aljoscha
left a comment
aljoscha
left a comment
There was a problem hiding this comment.
Approved, but I had some suggestions for wording.
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Enables to write metadata columns to a {@link DynamicTableSink}. |
There was a problem hiding this comment.
Maybe Interface for {@link DynamicTableSink DynamicTableSinks} that support writing metadata columns. sounds better.
| * <p>Metadata columns add additional columns to the table's schema. A table sink is responsible to | ||
| * accept requested metadata columns at the end of consumed rows and persist them. This includes potentially | ||
| * forwarding metadata columns to contained formats. |
There was a problem hiding this comment.
| * <p>Metadata columns add additional columns to the table's schema. A table sink is responsible to | |
| * accept requested metadata columns at the end of consumed rows and persist them. This includes potentially | |
| * forwarding metadata columns to contained formats. | |
| * <p>Metadata columns add additional columns to the table's schema. A table sink is responsible for | |
| * accepting requested metadata columns at the end of consumed rows and persist them. This includes potentially | |
| * forwarding metadata columns to contained formats. |
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Enables to read metadata columns from a {@link ScanTableSource}. |
There was a problem hiding this comment.
Maybe Interface for {@link ScanTableSource ScanTableSources} that support reading metadata columns. sounds better.
| * <p>Metadata columns add additional columns to the table's schema. A table source is responsible to | ||
| * add requested metadata columns at the end of produced rows. This includes potentially forwarding metadata | ||
| * columns from contained formats. |
There was a problem hiding this comment.
| * <p>Metadata columns add additional columns to the table's schema. A table source is responsible to | |
| * add requested metadata columns at the end of produced rows. This includes potentially forwarding metadata | |
| * columns from contained formats. | |
| * <p>Metadata columns add additional columns to the table's schema. A table source is responsible for | |
| * adding requested metadata columns at the end of produced rows. This includes potentially forwarding metadata | |
| * columns from contained formats. |
| * | ||
| * @see DecodingFormat#listReadableMetadata() | ||
| */ | ||
| Map<String, DataType> listReadableMetadata(); |
There was a problem hiding this comment.
Returning a Map might be a bit dangerous when the order is important. A List of tuples would seem better. But I'm sure this was discussed in the FLIP. So please ignore... 😅
There was a problem hiding this comment.
The order of the Map is not super important. In the end, the passed List decides about the order. But an implementation can sync both methods and avoid index mapping if desired.
4 participants
What is the purpose of the change
Adds all
@PublicEvolvinginterfaces mentioned in FLIP-107.Brief change log
EncodingFormatandDecodingFormatSupportsReadingMetadataandSupportsWritingMetadataVerifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): yesDocumentation