[core] add file-format-per-level for paimon#1500
Conversation
| private void updateFormatByLevel(int level) { | ||
| String fileFormat = levelFormats.get(level); | ||
| if (null != fileFormat) { | ||
| pathFactory.setFormatIdentifier(fileFormat); |
There was a problem hiding this comment.
Do not provide setFormatIdentifier, This is difficult to maintain.
A better way is to pass parameters when you need to do something, rather than setting its class variables.
There was a problem hiding this comment.
I udpate it by construct a new DataFilePathFactory
zhangjun0x01
force-pushed
the
file-format-per-level
branch
from
bf01d54 to
118f4dd
Compare
| private final RowType recordType; | ||
| private FormatWriterFactory writerFactory; | ||
| @Nullable private TableStatsExtractor tableStatsExtractor; | ||
| private DataFilePathFactory pathFactory; |
There was a problem hiding this comment.
Here we should maintain a Map<String, Factories> field to deal with different formats.
| suggestedFileSize); | ||
| } | ||
|
|
||
| private void updateFormatByLevel(int level) { |
There was a problem hiding this comment.
This style is not good because it dynamically decontaminates class member variables, which can lead to all sorts of problems.
In general, we recommend a functional style, which means that class members should ideally be immutable, with at most some cache stuff.
zhangjun0x01
force-pushed
the
file-format-per-level
branch
2 times, most recently
from
7969c33 to
91d2926
Compare
zhangjun0x01
force-pushed
the
file-format-per-level
branch
3 times, most recently
from
5187edd to
03e738d
Compare
zhangjun0x01
force-pushed
the
file-format-per-level
branch
from
03e738d to
d0a9b0f
Compare
There was a problem hiding this comment.
I understand that there is still some work needed here to make the code clearer, such as putting format related things into one class in write and introducing a LazyMap class to avoid creating all the structures. In addition, the handling of default format should also be placed inside this class, rather than outside as it is now.
|
ok，I will deal it later |
JingsongLi
force-pushed
the
file-format-per-level
branch
from
e28222f to
9bb8c5e
Compare
JingsongLi
force-pushed
the
file-format-per-level
branch
from
9bb8c5e to
ef1af77
Compare
|
I added a commit ef1af77 to improve code structure |
thank you very much . |
2 participants
Purpose
Linked issue: #1474
Tests
org.apache.paimon.flink.CatalogTableITCase#testFileFormatPerLevel
API and Format
Documentation