[Coral-Schema] Preserve precision in RelDataTypeToAvroType for TIMESTAMP, TIME, and fixed BINARY

[yyy1000]

Motivation

When RelToAvroSchemaConverter generates a view's Avro schema, pass-through columns reuse the base-table schema produced by MergeCoralSchemaWithAvro / MergeHiveSchemaWithAvro, while computed and aggregate expressions (e.g. MAX(col), COALESCE(col, …), CASE … END) have their Avro type derived from the Calcite RelDataType by RelDataTypeToAvroType (via SchemaUtilities.appendField).

That derivation was lossy for Iceberg-sourced types:

• TIMESTAMP was hardcoded to timestamp-millis, ignoring precision.
• TIME had no case at all — it threw UnsupportedOperationException.
• BINARY was always emitted as bytes.

Iceberg carries these precisely (TIMESTAMP(6), TIME, fixed BINARY(n)), and Calcite's aggregate / COALESCE / CASE return-type inference preserves the operand type. So a view that computes or aggregates over an Iceberg column hit the gap (pass-through of the same column is already correct via the merge engine):

Expression over an Iceberg base-table column	Before	After
MAX(time_col) / COALESCE(time_col, …)	throws UnsupportedOperationException	time-micros
MAX(ts6_col) / CASE … ts6_col … END	timestamp-millis (wrong)	timestamp-micros
MAX(fixed_col)	bytes (lossy)	fixed(n)

This also aligns the computed/aggregate path with the pass-through path, which MergeCoralSchemaWithAvro already emits as time-micros / timestamp-micros / fixed.

On TIME specifically: Hive has no TIME type, so a Calcite TIME originates only from the Iceberg/CoralCatalog path. That is why the previous default-case UnsupportedOperationException was never reached on the Hive path but is reachable for Iceberg-backed views.

Summary

basicSqlTypeToAvroType now derives the Avro logical type from the Calcite precision:

• TIMESTAMP precision > 3 (e.g. Iceberg's TIMESTAMP(6)) → timestamp-micros; unspecified precision and precision ≤ 3 → timestamp-millis.
• TIME → time-micros (previously unsupported).
• fixed-length BINARY(n) → Avro fixed; unbounded BINARY → bytes.

Hive behavior is unchanged

Hive TIMESTAMP, TIME, and BINARY resolve to PRECISION_NOT_SPECIFIED (HiveTypeSystem), so they keep their exact current output (timestamp-millis / bytes). Only precision-specified types — which originate from the Iceberg/CoralCatalog path — change. The existing testTimestampTypeField (a Hive view) still emits timestamp-millis.

Testing

• New RelDataTypeToAvroTypeTests.testPrecisionSensitiveTypes covering TIMESTAMP(6), unspecified TIMESTAMP, TIME, fixed BINARY(16), and unbounded BINARY, with golden rel2avro-testPrecisionTypes-expected.avsc.
• Full coral-schema suite green (143 tests).

[simbadzina]

Consider citing the Iceberg spec for the microsecond-precision claim so the invariant is traceable:

time — Time of day, microsecond precision, without date, timezone
https://github.com/apache/iceberg/blob/8f28a86914d6beaf2615cdf5797da0acb72ea4d9/format/spec.md?plain=1#L273

[Copilot]

Pull request overview

This PR updates Coral Schema’s RelDataTypeToAvroType conversion so computed/aggregate Avro schema generation preserves precision-sensitive Iceberg-derived types more accurately.

Changes:

• Maps TIMESTAMP precision > 3 to timestamp-micros while preserving existing unspecified/≤3 behavior as timestamp-millis.
• Adds TIME conversion to Avro time-micros.
• Emits Avro fixed(n) for precision-specified BINARY(n) and adds coverage with a new expected schema fixture.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
coral-schema/src/main/java/com/linkedin/coral/schema/avro/RelDataTypeToAvroType.java	Adds precision-aware Avro mappings for timestamp, time, and fixed binary types.
coral-schema/src/test/java/com/linkedin/coral/schema/avro/RelDataTypeToAvroTypeTests.java	Adds a direct RelDataType test covering precision-sensitive conversions.
coral-schema/src/test/resources/rel2avro-testPrecisionTypes-expected.avsc	Provides the golden Avro schema for the new precision-sensitive type test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[simbadzina]

Not blocking, but worth noting: this defaults unspecified-precision TIMESTAMP to timestamp-millis, while MergeCoralSchemaWithAvro.timestampToAvro defaults it to timestamp-micros. They can't collide today (Iceberg is always TIMESTAMP(6), and Hive tables route to MergeHiveSchemaWithAvro), but if we ever move Hive onto MergeCoralSchemaWithAvro the defaults would disagree. Maybe a short comment on each so a future consolidation reconciles them on purpose.

[simbadzina]

nit: could we wrap the two comparisons in parens — ((getPrecision() != PRECISION_NOT_SPECIFIED) && (getPrecision() > 3))? Reads clearer than leaning on != / && precedence. Same for the BINARY case above.

[simbadzina]

We could also just have getPrecision() > 3 since PRECISION_NOT_SPECIFIED is -1.

[simbadzina]

Generally LGTM. Left a few comments for code cleanup.

Waiting for author to respond to review comments.

[yyy1000]

Thanks @simbadzina! All addressed in the latest commit:

• Simplified both guards to getPrecision() > 3 (timestamp) and > 0 (binary) — since PRECISION_NOT_SPECIFIED == -1, the explicit != was redundant.
• Cited the Iceberg spec inline on the TIME case (microsecond precision).
• Added a note on the unspecified-precision default divergence on both RelDataTypeToAvroType and MergeCoralSchemaWithAvro.timestampToAvro, so a future Hive-on-Coral consolidation reconciles them deliberately.

[simbadzina]

LGTM

[aastha25]

thanks for working on this PR, looks like something that showed up in i-testing :)
LGTM,
2 questions
(1) for computed and aggregate expressions, how does the RelDatatype get the column casing? which is the exact metadata, for HMS & Iceberg, it refers to and can you also please link the code path? might exist in calcite but derivable from coral code.
(2) I noted that RelDataTypeToAvroType is creating Schema without nullability information. (2a) where is the nullability from the base table column superimposed in the view avro schema. (2b) if its a separate class / visitor, do you see value in bringing it here in the same class and converging the logic?

[yyy1000]

Thanks @aastha25 — yep, it came out of i-testing :) Two good questions:

(1) Column casing / metadata. RelDataTypeToAvroType is casing-neutral — it emits the RelNode field name verbatim (relRecordTypeToAvroType → toAvroQualifiedName(relField.getName()), which only maps $→_). The name is fixed upstream when HiveToRelConverter builds the RelNode:

• Computed / aggregate output columns — the name is the project's RelNode field name: RelToAvroSchemaConverter#visit(LogicalProject) collects logicalProject.getRowType().getFieldList()[i].getName() (RelToAvroSchemaConverter.java:254-255). That is the SQL alias as written (or Calcite's derived name), not the base column.
• Base-column references — casing comes from the table metadata:
  • HMS: HiveCalciteTableAdapter#getRowTypeFromHiveType reads FieldSchema.getName() (:234); HMS stores column names lower-cased.
  • Iceberg: the Iceberg schema's NestedField.name(), preserved case-sensitively by IcebergToCoralTypeConverter#convert (:45/:178) → CoralTypeToRelDataTypeConverter.

So this PR is casing-neutral; it only changes leaf logical types.

(2) Nullability. Correct — relDataTypeToAvroTypeNonNullable intentionally returns a non-nullable schema (per its javadoc: "The return schema is always non nullable ... since the nullability if decided out of this method").

• (2a) Nullability is superimposed by the caller in SchemaUtilities#appendField(..., isNullable), which wraps the type as [null, schema]. The isNullable is decided per field in SchemaRexShuttle: computed RexCall → SchemaUtilities.isFieldNullable(rexCall, inputSchema) (RelToAvroSchemaConverter.java:499-501); pass-through RexInputRef → reuses the input-schema field, so nullability flows from the base-table merge engine (MergeCoralSchemaWithAvro / MergeHiveSchemaWithAvro — Iceberg required/optional or Hive partner Avro) via appendRexInputRefField (:459); aggregate calls → the aggregate's RelDataType (:351).
• (2b) The split is deliberate: RelDataTypeToAvroType is a pure RelDataType → Avro type mapper, while nullability is contextual (base-table optionality, aggregate-over-empty-group, literals) and lives where the shuttle has that context. Converging it would mean threading field nullability into RelDataTypeToAvroType — a reasonable follow-up (the relDataTypeToAvroType wrapper already carries a TODO around :85), but orthogonal to this precision-only change. Happy to file an issue to track that convergence.

For confidence I ran this through the i-testing parity harness against ~4k production views (each view's Avro schema generated via both the Hive and CoralCatalog paths, then diffed): no regression traces to this PR — its only effect is 60 views moving timestamp-millis → timestamp-micros, exactly as intended. The run also confirmed the Iceberg path emits Iceberg-accurate required/optional per the design's source-of-truth rule (relevant to your (2)), and surfaced a few multi-branch-union gaps that are PR4's scope, not this PR.