JingsongLi commented on code in PR #646:
URL: https://github.com/apache/incubator-paimon/pull/646#discussion_r1142151726
##########
paimon-format/src/main/java/org/apache/paimon/format/orc/OrcFileFormat.java:
##########
@@ -63,13 +62,15 @@ public class OrcFileFormat extends FileFormat {
private final Properties orcProperties;
private final org.apache.hadoop.conf.Configuration readerConf;
private final org.apache.hadoop.conf.Configuration writerConf;
+ private final int readBatchSize;
- public OrcFileFormat(Options formatOptions) {
+ public OrcFileFormat(Options formatOptions, int readBatchSize) {
Review Comment:
We can just create constructor of `FormatContext`.
We can create a `FormatContext.copy(Options newOptions)`.
##########
paimon-core/src/main/java/org/apache/paimon/CoreOptions.java:
##########
@@ -573,6 +580,15 @@ public String partitionDefaultName() {
return options.get(PARTITION_DEFAULT_NAME);
}
+ public static FileFormat fromTableOptions(Options options,
ConfigOption<String> formatOption) {
Review Comment:
`createFileFormat`, this is in `CoreOptions` now.
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetFileFormat.java:
##########
@@ -38,10 +38,12 @@
public class ParquetFileFormat extends FileFormat {
private final Options formatOptions;
+ private final int readBatchSize;
- public ParquetFileFormat(Options formatOptions) {
+ public ParquetFileFormat(Options formatOptions, int readBatchSize) {
Review Comment:
ditto
##########
paimon-common/src/main/java/org/apache/paimon/format/FileFormat.java:
##########
@@ -99,12 +98,17 @@ public static FileFormat fromIdentifier(String identifier,
Options options) {
}
private static Optional<FileFormat> fromIdentifier(
- String formatIdentifier, Options formatOptions, ClassLoader
classLoader) {
+ String formatIdentifier,
+ FileFormatFactory.FormatContext context,
+ ClassLoader classLoader) {
ServiceLoader<FileFormatFactory> serviceLoader =
ServiceLoader.load(FileFormatFactory.class, classLoader);
for (FileFormatFactory factory : serviceLoader) {
if (factory.identifier().equals(formatIdentifier.toLowerCase())) {
- return Optional.of(factory.create(formatOptions));
+ return Optional.of(
+ factory.create(
+ new FileFormatFactory.FormatContext(
Review Comment:
why need to copy `FormatContext `?
##########
paimon-common/src/main/java/org/apache/paimon/format/FileFormat.java:
##########
@@ -75,20 +75,19 @@ public Optional<FileStatsExtractor>
createStatsExtractor(RowType type) {
return Optional.empty();
}
- /** Create a {@link FileFormat} from table options. */
- public static FileFormat fromTableOptions(
- Options tableOptions, ConfigOption<String> formatOption) {
- String formatIdentifier = tableOptions.get(formatOption);
- return fromIdentifier(formatIdentifier,
tableOptions.removePrefix(formatIdentifier + "."));
+ @VisibleForTesting
+ public static FileFormat fromIdentifier(String identifier, Options
options) {
+ return fromIdentifier(identifier, new
FileFormatFactory.FormatContext(options, 1024));
}
/** Create a {@link FileFormat} from format identifier and format options.
*/
- public static FileFormat fromIdentifier(String identifier, Options
options) {
+ public static FileFormat fromIdentifier(
Review Comment:
import `FileFormatFactory.FormatContext`
--
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]
