Make GenericRecord and ClickHouseBinaryFormatReader easy to iterate#2004
Make GenericRecord and ClickHouseBinaryFormatReader easy to iterate#2004chernser merged 2 commits intoClickHouse:mainfrom
Conversation
abcfy2
force-pushed
the
main
branch
2 times, most recently
from
e2242b6 to
74aef04
Compare
|
|
||
| @Override | ||
| public int getColumnCount() { | ||
| return currentRecord.size(); |
There was a problem hiding this comment.
It should be returned from schema because it may be a virtual columns (for example, when we do so for tuples)
|
|
||
| @Override | ||
| public Set<String> getColumnNames() { | ||
| return currentRecord.keySet(); |
There was a problem hiding this comment.
It should come from schema because it may have additional columns.
|
|
||
| @Override | ||
| public Map<String, Object> current() { | ||
| return currentRecord; |
There was a problem hiding this comment.
This cannot be exposed because it is internal records and should not be accessed in such way. I plan to change internal later.
|
@abcfy2 Thank you for contribution! |
|
@chernser Thanks, please check again. 😃 I've never thought to use |
|
So the new use case should be: records.forEach(record -> {
TableSchema schema = record.getSchema();
// easy to get column size, then user can use record.getObject(<index>) to get a specified column value
System.out.println(schema.getColumns().size());
// easy to get column names, then user can use record.getObject(<column_name>) to get a specified column value
System.out.println(schema.getColumns().stream().map(ClickHouseColumn::getColumnName).toList());
// easy to get all row values in kv format
System.out.println(record.getAll());
}); |
abcfy2
force-pushed
the
main
branch
2 times, most recently
from
376ca8c to
bd363cf
Compare
| * | ||
| * @return a Map of column names and values. | ||
| */ | ||
| default Map<String, Object> getAll() { |
There was a problem hiding this comment.
lets rename it to getValues or asMap.
I would prefer having no default implementation in interfaces because default was designed to not break things when new stream API was introduced in java. I think, that only this case is valid usage of default.
|
@abcfy2 than you for changing the PR! Thanks! |
|
No worries @chernser . I've modified PR. Please check again. Thanks. |
|
@abcfy2 looks good to me! |
|
🤨 Some tests failed. But seems it's non of my mistake. |
2 participants
Summary
Add some useful methods to
GenericRecordandClickHouseBinaryFormatReaderto make it easy to iterate.Both
GenericRecordandClickHouseBinaryFormatReadercould also get the column size, column names, and the full row values.Use case: