Skip to content

[WIP] Support for 'time' type in Iceberg, reading only #1546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ianton-ru wants to merge 5 commits into
base: antalya-25.8
Choose a base branch
from
Open

[WIP] Support for 'time' type in Iceberg, reading only #1546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d88f5e4
Iceberg type 'time' support
ianton-ru Mar 18, 2026
7138d5c
Merge branch 'bugfix/antalya-25.8/1536_fix_wrong_path_decodig' into test
ianton-ru Mar 18, 2026
94a1346
Fix tests
ianton-ru Mar 19, 2026
0c56e3c
Fix
ianton-ru Mar 19, 2026
60f133d
Merge branch 'antalya-25.8' into bugfix/antalya-25.8/1535_time_type_r…
ianton-ru Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/Processors/Formats/Impl/AvroRowInputFormat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ static void insertNumber(IColumn & column, WhichDataType type, T value)
case TypeIndex::DateTime64:
assert_cast<ColumnDecimal<DateTime64> &>(column).insertValue(static_cast<Int64>(value));
break;
case TypeIndex::Time64:
assert_cast<ColumnDecimal<Time64> &>(column).insertValue(static_cast<Int64>(value));
break;
case TypeIndex::IPv4:
assert_cast<ColumnIPv4 &>(column).insertValue(IPv4(static_cast<UInt32>(value)));
break;
Expand Down Expand Up @@ -303,6 +306,8 @@ AvroDeserializer::DeserializeFn AvroDeserializer::createDeserializeFn(const avro
return createDecimalDeserializeFn<DataTypeDecimal256>(root_node, target_type, false);
if (target.isDateTime64())
return createDecimalDeserializeFn<DataTypeDateTime64>(root_node, target_type, false);
if (target.isTime64())
return createDecimalDeserializeFn<DataTypeTime64>(root_node, target_type, false);
break;
case avro::AVRO_INT:
if (target_type->isValueRepresentedByNumber())
Expand Down Expand Up @@ -1282,8 +1287,11 @@ DataTypePtr AvroSchemaReader::avroNodeToDataType(avro::NodePtr node)
{
case avro::Type::AVRO_INT:
{
if (node->logicalType().type() == avro::LogicalType::DATE)
auto logical_type = node->logicalType();
if (logical_type.type() == avro::LogicalType::DATE)
return {std::make_shared<DataTypeDate32>()};
if (logical_type.type() == avro::LogicalType::TIME_MILLIS)
return {std::make_shared<DataTypeTime64>(3)};

return {std::make_shared<DataTypeInt32>()};
}
Expand All @@ -1294,6 +1302,10 @@ DataTypePtr AvroSchemaReader::avroNodeToDataType(avro::NodePtr node)
return {std::make_shared<DataTypeDateTime64>(3)};
if (logical_type.type() == avro::LogicalType::TIMESTAMP_MICROS)
return {std::make_shared<DataTypeDateTime64>(6)};
if (logical_type.type() == avro::LogicalType::TIME_MILLIS)
return {std::make_shared<DataTypeTime64>(3)};
if (logical_type.type() == avro::LogicalType::TIME_MICROS)
return {std::make_shared<DataTypeTime64>(6)};

return std::make_shared<DataTypeInt64>();
}
Expand Down
4 changes: 4 additions & 0 deletions src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ bool canDumpIcebergStats(const Field & field, DataTypePtr type)
case TypeIndex::Date32:
case TypeIndex::Int64:
case TypeIndex::DateTime64:
case TypeIndex::Time:
case TypeIndex::Time64:
case TypeIndex::String:
return true;
default:
Expand All @@ -143,7 +145,9 @@ std::vector<uint8_t> dumpFieldToBytes(const Field & field, DataTypePtr type)
case TypeIndex::Date32:
return dumpValue(field.safeGet<Int32>());
case TypeIndex::Int64:
case TypeIndex::Time:
return dumpValue(field.safeGet<Int64>());
Comment on lines +148 to 149
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize Time64 stats via decimal payload

TypeIndex::Time64 values are stored as decimal fields, but this branch reads them with field.safeGet<Int64>(). Since canDumpIcebergStats() now explicitly allows Time64, any Iceberg write that emits stats for a Time64 column can fail at runtime with a field type mismatch when building lower/upper bounds. This should use the same decimal extraction path as DateTime64 (or an equivalent conversion) instead of the integer accessor.

Useful? React with 👍 / 👎.

case TypeIndex::Time64:
case TypeIndex::DateTime64:
return dumpValue(field.safeGet<Decimal64>().getValue().value);
Comment on lines +148 to 152
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize Time stats using Int64 field type

canDumpIcebergStats now allows TypeIndex::Time, but dumpFieldToBytes handles that branch with field.safeGet<Decimal64>(); Time values are stored in Field as Int64, so this path throws BAD_GET when lower/upper bounds are written. In practice, inserts into Iceberg tables that include a Time column can now fail during manifest statistics serialization instead of just skipping stats for that type.

Useful? React with 👍 / 👎.

Comment on lines +148 to 152
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize Iceberg time bounds in microseconds

dumpFieldToBytes now emits raw ClickHouse storage units for Time/Time64 into manifest lower_bounds/upper_bounds, but Iceberg time bounds are defined as microseconds-from-midnight. Here Time is written as integer seconds and Time64 is written as raw decimal ticks, so values are mis-scaled unless they are already microseconds; e.g. Time and Time64(3) produce smaller bounds by 1e6/1e3. Any reader that trusts bounds for predicate pruning can incorrectly skip matching files and return incomplete results. Please rescale to microseconds (or skip writing bounds for non-microsecond scales) before serializing.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

@ianton-ru ianton-ru Mar 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so values are mis-scaled unless they are already microseconds

Time value from Iceberg always has microseconds, see spec - https://iceberg.apache.org/spec/#primitive-types

case TypeIndex::String:
Expand Down
2 changes: 1 addition & 1 deletion src/Storages/ObjectStorage/DataLakes/Iceberg/SchemaProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ DataTypePtr IcebergSchemaProcessor::getSimpleType(const String & type_name, Cont
if (type_name == f_date)
return std::make_shared<DataTypeDate>();
if (type_name == f_time)
return std::make_shared<DataTypeInt64>();
return std::make_shared<DataTypeTime64>(6);
if (type_name == f_timestamp)
return std::make_shared<DataTypeDateTime64>(6);
if (type_name == f_timestamptz)
Expand Down
1 change: 1 addition & 0 deletions src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ std::pair<Poco::Dynamic::Var, bool> getIcebergType(DataTypePtr type, Int32 & ite
case TypeIndex::DateTime64:
return {"timestamp", true};
case TypeIndex::Time:
case TypeIndex::Time64:
return {"time", true};
case TypeIndex::String:
return {"string", true};
Expand Down
14 changes: 9 additions & 5 deletions tests/integration/test_database_iceberg/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,9 +982,7 @@ def test_partitioning_by_time(started_cluster, storage_type):

create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME)

# Fix test when https://github.com/Altinity/ClickHouse/issues/15355 is resolved
# Must be 43200
assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{namespace}.{table_name}`") == "43200000000\ttest\n"
assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{namespace}.{table_name}`") == "12:00:00.000000\ttest\n"


@pytest.mark.parametrize("storage_type", ["s3"])
Expand Down Expand Up @@ -1012,6 +1010,12 @@ def test_partitioning_by_string(started_cluster, storage_type):
field_type=StringType(),
required=False,
),
NestedField(
field_id=3,
name="time_value",
field_type=TimeType(),
required=False,
),
)

partition_spec = PartitionSpec(
Expand All @@ -1021,10 +1025,10 @@ def test_partitioning_by_string(started_cluster, storage_type):
)

table = create_table(catalog, namespace, table_name, schema=schema, partition_spec=partition_spec)
data = [{"key": "a:b,c[d=e/f%g?h", "value": "test"}]
data = [{"key": "a:b,c[d=e/f%g?h", "value": "test", "time_value": dtime(12,0,0)}]
df = pa.Table.from_pylist(data)
table.append(df)

create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME)

assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{namespace}.{table_name}`") == "a:b,c[d=e/f%g?h\ttest\n"
assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{namespace}.{table_name}`") == "a:b,c[d=e/f%g?h\ttest\t12:00:00.000000\n"
Loading