This is an automated email from the ASF dual-hosted git repository.

Fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-java.git


The following commit(s) were added to refs/heads/master by this push:
     new ac007fe19 PARQUET-2249: Write IEEE 754 total order by default (#3699)
ac007fe19 is described below

commit ac007fe19f9426dca3e3158fec217c9ca7af320f
Author: Jiayi Wang <[email protected]>
AuthorDate: Thu Aug 6 21:11:32 2026 +0200

    PARQUET-2249: Write IEEE 754 total order by default (#3699)
    
    * PARQUET-2249: Write IEEE 754 total order by default for floating-point 
columns
    
    Follow-up to #3393, which added IEEE_754_TOTAL_ORDER support but kept
    TYPE_DEFINED_ORDER as the default for FLOAT, DOUBLE and FLOAT16 columns.
    
    Keeping the type-defined order as the default is a latent backward-compat
    hazard: the writer now computes finite min/max over the non-NaN subset and
    records nan_count, but an old reader that predates nan_count ignores it,
    accepts the finite bounds, and can incorrectly prune row groups that contain
    NaN. Readers instead ignore statistics written under an unknown sort order, 
so
    writing IEEE 754 total order by default is the safer behavior. See the
    discussion on #3393.
    
    This makes FLOAT, DOUBLE and FLOAT16 columns built without an explicit 
column
    order default to IEEE_754_TOTAL_ORDER (mirroring how #3610 defaults INT96 to
    INT96_TIMESTAMP_ORDER). Columns with a logical annotation that does not 
accept
    IEEE 754 total order (e.g. an unknown annotation) fall back to type-defined
    order so they remain constructible. The default-order selection is unified 
in
    PrimitiveType.defaultColumnOrder so construction and text serialization 
agree.
    
    To stay backward compatible on read, a footer that carries no column_orders
    list predates IEEE_754_TOTAL_ORDER, so floating-point columns read from 
such a
    footer are given type-defined order rather than inheriting the new
    construction-time default; their legacy statistics are thus not 
reinterpreted
    under IEEE 754 total order.
    
    The text schema representation now carries a non-default column order
    (columnorder(...) after the type/annotation) and MessageTypeParser parses 
it,
    so a column order set explicitly survives toString()/parse round-trips such 
as
    the one GroupWriteSupport performs. Columns left at their default emit no 
token,
    keeping existing schema strings unchanged.
    
    Tests that exercise the legacy type-defined NaN / +-0 semantics set
    TYPE_DEFINED_ORDER explicitly, and new tests cover the default 
serialization,
    the column-order-less read path, and the text round-trip.
    
    Co-authored-by: Isaac
    
    * Degrade unrecognized text-schema column order to UNDEFINED
    
    Address review feedback: MessageTypeParser.parseColumnOrder now returns
    ColumnOrder.undefined() for a columnorder(...) token it does not recognize,
    instead of throwing. This matches 
ParquetMetadataConverter.fromParquetColumnOrder
    ("not yet supported by this API") so a schema string written by a newer API 
with
    an order this version does not know stays parseable; statistics under an 
unknown
    order are ignored by readers anyway. UNDEFINED is a valid order for all 
primitive
    types, so it is safe to feed into the builder.
    
    Co-authored-by: Isaac
    
    * Reconcile differing column orders as UNDEFINED when merging schemas
    
    Address review follow-up (codex via @gszadovszky): with floats defaulting to
    IEEE_754_TOTAL_ORDER and legacy footers read as TYPE_DEFINED_ORDER, 
aggregating
    footers over a directory that spans the upgrade (e.g. ParquetInputFormat 
split
    planning / getGlobalMetaData, or the deprecated summary-file merge) threw
    IncompatibleSchemaModificationException on the otherwise-identical float 
column.
    
    PrimitiveType.union now reconciles a column-order-only difference to 
UNDEFINED
    instead of failing. At that point type, logical type and length already 
match, so
    the order is the only difference and the columns are otherwise mergeable. 
This is
    safe because per-file statistics are still read under each file's own 
column order
    (from its own footer); only the merged schema's ambiguous ordering claim is
    dropped. Added TestMessageType.testMergeMixedFloatingColumnOrder and updated
    testMergeSchemaWithColumnOrder, which previously asserted the merge threw.
    
    Co-authored-by: Isaac
---
 .../apache/parquet/schema/MessageTypeParser.java   | 20 ++++++
 .../org/apache/parquet/schema/PrimitiveType.java   | 59 ++++++++++++-----
 .../main/java/org/apache/parquet/schema/Types.java |  8 ++-
 .../parquet/column/statistics/TestStatistics.java  |  7 +-
 .../column/statistics/TestStatisticsNanCount.java  | 11 ++--
 .../column/columnindex/TestColumnIndexBuilder.java |  7 +-
 .../columnindex/TestColumnIndexBuilderNaN.java     | 11 ++--
 .../org/apache/parquet/schema/TestMessageType.java | 74 ++++++++++++++++++++--
 .../format/converter/ParquetMetadataConverter.java |  7 ++
 .../dictionarylevel/DictionaryFilterTest.java      |  9 ++-
 .../converter/TestParquetMetadataConverter.java    | 70 +++++++++++++++++++-
 .../FloatingPointNanInteropFileGenerator.java      |  3 +
 .../statistics/TestFloat16ReadWriteRoundTrip.java  |  3 +
 .../parquet/statistics/TestFloat16Statistics.java  |  3 +
 14 files changed, 255 insertions(+), 37 deletions(-)

diff --git 
a/parquet-column/src/main/java/org/apache/parquet/schema/MessageTypeParser.java 
b/parquet-column/src/main/java/org/apache/parquet/schema/MessageTypeParser.java
index c7f4b900b..6ccdc4ca7 100644
--- 
a/parquet-column/src/main/java/org/apache/parquet/schema/MessageTypeParser.java
+++ 
b/parquet-column/src/main/java/org/apache/parquet/schema/MessageTypeParser.java
@@ -225,6 +225,12 @@ public class MessageTypeParser {
       check(t, ")", "logical type ended by )", st);
       t = st.nextToken();
     }
+    if (t.equalsIgnoreCase(PrimitiveType.COLUMN_ORDER_KEYWORD)) {
+      check(st.nextToken(), "(", "column order followed by (", st);
+      childBuilder.columnOrder(parseColumnOrder(st.nextToken()));
+      check(st.nextToken(), ")", "column order ended by )", st);
+      t = st.nextToken();
+    }
     if (t.equals("=")) {
       childBuilder.id(Integer.parseInt(st.nextToken()));
       t = st.nextToken();
@@ -240,6 +246,20 @@ public class MessageTypeParser {
     }
   }
 
+  private static ColumnOrder parseColumnOrder(String t) {
+    // An unrecognized order degrades to UNDEFINED rather than failing, 
matching
+    // ParquetMetadataConverter.fromParquetColumnOrder, so a schema written by 
a newer API with an
+    // order this version does not know stays parseable. Statistics under an 
unknown order are
+    // ignored by readers anyway.
+    if 
(t.equalsIgnoreCase(ColumnOrder.ColumnOrderName.TYPE_DEFINED_ORDER.name())) {
+      return ColumnOrder.typeDefined();
+    }
+    if 
(t.equalsIgnoreCase(ColumnOrder.ColumnOrderName.IEEE_754_TOTAL_ORDER.name())) {
+      return ColumnOrder.ieee754TotalOrder();
+    }
+    return ColumnOrder.undefined();
+  }
+
   private static boolean isLogicalType(String t) {
     return Arrays.stream(LogicalTypeAnnotation.LogicalTypeToken.values())
         .anyMatch((type) -> type.name().equals(t));
diff --git 
a/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java 
b/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java
index c3766fa15..e04c54a7d 100644
--- a/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java
+++ b/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java
@@ -492,6 +492,9 @@ public final class PrimitiveType extends Type {
     }
   }
 
+  // Keyword used to render/parse a non-default column order in the text 
schema representation.
+  static final String COLUMN_ORDER_KEYWORD = "columnorder";
+
   private final PrimitiveTypeName primitive;
   private final int length;
   private final DecimalMetadata decimalMeta;
@@ -578,9 +581,7 @@ public final class PrimitiveType extends Type {
     this.decimalMeta = decimalMeta;
 
     if (columnOrder == null) {
-      columnOrder = primitive == PrimitiveTypeName.INT96 || originalType == 
OriginalType.INTERVAL
-          ? ColumnOrder.undefined()
-          : ColumnOrder.typeDefined();
+      columnOrder = defaultColumnOrder(primitive, originalType, 
getLogicalTypeAnnotation());
     } else if (columnOrder.getColumnOrderName() == 
ColumnOrderName.IEEE_754_TOTAL_ORDER) {
       Preconditions.checkArgument(
           primitive == PrimitiveTypeName.FLOAT || primitive == 
PrimitiveTypeName.DOUBLE,
@@ -629,10 +630,7 @@ public final class PrimitiveType extends Type {
     }
 
     if (columnOrder == null) {
-      columnOrder = primitive == PrimitiveTypeName.INT96
-              || logicalTypeAnnotation instanceof 
LogicalTypeAnnotation.IntervalLogicalTypeAnnotation
-          ? ColumnOrder.undefined()
-          : ColumnOrder.typeDefined();
+      columnOrder = defaultColumnOrder(primitive, getOriginalType(), 
logicalTypeAnnotation);
     } else if (columnOrder.getColumnOrderName() == 
ColumnOrderName.IEEE_754_TOTAL_ORDER) {
       Preconditions.checkArgument(
           primitive == PrimitiveTypeName.FLOAT
@@ -648,6 +646,27 @@ public final class PrimitiveType extends Type {
     this.columnOrder = requireValidColumnOrder(columnOrder);
   }
 
+  /**
+   * The column order used when none is specified explicitly. INT96 and 
INTERVAL have no defined
+   * ordering, so they default to undefined. Floating-point types default to 
IEEE 754 total order so
+   * that NaN values and the sign of zero are ordered deterministically and 
nan_count statistics can
+   * be written; this is skipped when the logical type annotation does not 
accept IEEE 754 total
+   * order (e.g. an unknown annotation), leaving the type constructible with 
the type-defined order.
+   */
+  private static ColumnOrder defaultColumnOrder(
+      PrimitiveTypeName primitive, OriginalType originalType, 
LogicalTypeAnnotation logicalTypeAnnotation) {
+    if (primitive == PrimitiveTypeName.INT96 || originalType == 
OriginalType.INTERVAL) {
+      return ColumnOrder.undefined();
+    }
+    boolean isFloatingType = primitive == PrimitiveTypeName.FLOAT
+        || primitive == PrimitiveTypeName.DOUBLE
+        || (logicalTypeAnnotation != null
+            && logicalTypeAnnotation.getType() == 
LogicalTypeAnnotation.LogicalTypeToken.FLOAT16);
+    boolean acceptsIeee754 = logicalTypeAnnotation == null
+        || 
logicalTypeAnnotation.isValidColumnOrder(ColumnOrder.ieee754TotalOrder());
+    return isFloatingType && acceptsIeee754 ? ColumnOrder.ieee754TotalOrder() 
: ColumnOrder.typeDefined();
+  }
+
   private ColumnOrder requireValidColumnOrder(ColumnOrder columnOrder) {
     if (primitive == PrimitiveTypeName.INT96) {
       Preconditions.checkArgument(
@@ -748,6 +767,13 @@ public final class PrimitiveType extends Type {
       // TODO: should we print decimal metadata too?
       sb.append(" 
(").append(getLogicalTypeAnnotation().toString()).append(")");
     }
+    // Only emit the column order when it differs from the default, so schemas 
that rely on the
+    // default stay textually unchanged.
+    if (!columnOrder.equals(defaultColumnOrder(primitive, getOriginalType(), 
getLogicalTypeAnnotation()))) {
+      sb.append(" ").append(COLUMN_ORDER_KEYWORD).append("(");
+      sb.append(columnOrder.getColumnOrderName().name());
+      sb.append(")");
+    }
     if (getId() != null) {
       sb.append(" = ").append(getId());
     }
@@ -857,17 +883,13 @@ public final class PrimitiveType extends Type {
     throw new IncompatibleSchemaModificationException("can not merge type " + 
toMerge + " into " + this);
   }
 
-  private void reportSchemaMergeErrorWithColumnOrder(Type toMerge) {
-    throw new IncompatibleSchemaModificationException("can not merge type " + 
toMerge + " with column order "
-        + toMerge.asPrimitiveType().columnOrder() + " into " + this + " with 
column order " + columnOrder());
-  }
-
   @Override
   protected Type union(Type toMerge, boolean strict) {
     if (!toMerge.isPrimitive()) {
       reportSchemaMergeError(toMerge);
     }
 
+    ColumnOrder mergedColumnOrder = columnOrder();
     if (strict) {
       // Can't merge primitive fields of different type names or different 
original types
       if (!primitive.equals(toMerge.asPrimitiveType().getPrimitiveTypeName())
@@ -881,9 +903,14 @@ public final class PrimitiveType extends Type {
         reportSchemaMergeError(toMerge);
       }
 
-      // Can't merge primitive fields with different column orders
+      // A column-order difference is the only remaining difference here 
(type, logical type and
+      // length already match). Reconcile to UNDEFINED instead of failing the 
merge: it lets an
+      // aggregation over otherwise-identical files with different column 
orders succeed -- e.g. a
+      // pre-upgrade float footer read as TYPE_DEFINED_ORDER merged with a 
post-upgrade one written
+      // as IEEE_754_TOTAL_ORDER. Per-file statistics are still read under 
each file's own column
+      // order, so this only drops the merged schema's (now ambiguous) 
ordering claim.
       if (!columnOrder().equals(toMerge.asPrimitiveType().columnOrder())) {
-        reportSchemaMergeErrorWithColumnOrder(toMerge);
+        mergedColumnOrder = ColumnOrder.undefined();
       }
     }
 
@@ -894,7 +921,9 @@ public final class PrimitiveType extends Type {
       builder.length(length);
     }
 
-    return 
builder.as(getLogicalTypeAnnotation()).columnOrder(columnOrder()).named(getName());
+    return builder.as(getLogicalTypeAnnotation())
+        .columnOrder(mergedColumnOrder)
+        .named(getName());
   }
 
   /**
diff --git a/parquet-column/src/main/java/org/apache/parquet/schema/Types.java 
b/parquet-column/src/main/java/org/apache/parquet/schema/Types.java
index 2f12991ab..ad550cbc7 100644
--- a/parquet-column/src/main/java/org/apache/parquet/schema/Types.java
+++ b/parquet-column/src/main/java/org/apache/parquet/schema/Types.java
@@ -414,9 +414,11 @@ public class Types {
     /**
      * Adds the column order for the primitive type.
      * <p>
-     * In case of not set the default column order is {@link 
ColumnOrderName#TYPE_DEFINED_ORDER} except the type
-     * {@link PrimitiveTypeName#INT96} and the types annotated by {@link 
OriginalType#INTERVAL} where the default column
-     * order is {@link ColumnOrderName#UNDEFINED}.
+     * In case of not set the default column order is {@link 
ColumnOrderName#TYPE_DEFINED_ORDER}, with the following
+     * exceptions: the floating-point types {@link PrimitiveTypeName#FLOAT}, 
{@link PrimitiveTypeName#DOUBLE} and the
+     * {@code FLOAT16} logical type default to {@link 
ColumnOrderName#IEEE_754_TOTAL_ORDER}; the type
+     * {@link PrimitiveTypeName#INT96} and the types annotated by {@link 
OriginalType#INTERVAL} default to
+     * {@link ColumnOrderName#UNDEFINED}.
      *
      * @param columnOrder the column order for the primitive type
      * @return this builder for method chaining
diff --git 
a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java
 
b/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java
index b04f84938..92c5cfed0 100644
--- 
a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java
+++ 
b/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java
@@ -38,6 +38,7 @@ import static org.assertj.core.data.Offset.offset;
 import java.nio.ByteBuffer;
 import java.util.Locale;
 import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.ColumnOrder;
 import org.apache.parquet.schema.OriginalType;
 import org.apache.parquet.schema.PrimitiveType;
 import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
@@ -775,7 +776,8 @@ public class TestStatistics {
 
   @Test
   public void testSpecBuilderForFloat() {
-    PrimitiveType type = Types.required(FLOAT).named("test_float");
+    PrimitiveType type =
+        
Types.required(FLOAT).columnOrder(ColumnOrder.typeDefined()).named("test_float");
     Statistics.Builder builder = Statistics.getBuilderForReading(type);
     Statistics<?> stats = 
builder.withMin(intToBytes(floatToIntBits(Float.NaN)))
         .withMax(intToBytes(floatToIntBits(42.0f)))
@@ -839,7 +841,8 @@ public class TestStatistics {
 
   @Test
   public void testSpecBuilderForDouble() {
-    PrimitiveType type = Types.required(DOUBLE).named("test_double");
+    PrimitiveType type =
+        
Types.required(DOUBLE).columnOrder(ColumnOrder.typeDefined()).named("test_double");
     Statistics.Builder builder = Statistics.getBuilderForReading(type);
     Statistics<?> stats = 
builder.withMin(longToBytes(doubleToLongBits(Double.NaN)))
         .withMax(longToBytes(doubleToLongBits(42.0)))
diff --git 
a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatisticsNanCount.java
 
b/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatisticsNanCount.java
index 2e09537b8..4b9e20a6a 100644
--- 
a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatisticsNanCount.java
+++ 
b/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatisticsNanCount.java
@@ -32,13 +32,16 @@ import org.junit.jupiter.api.Test;
 
 public class TestStatisticsNanCount {
 
-  private static final PrimitiveType FLOAT_TYPE =
-      Types.optional(PrimitiveTypeName.FLOAT).named("test_float");
-  private static final PrimitiveType DOUBLE_TYPE =
-      Types.optional(PrimitiveTypeName.DOUBLE).named("test_double");
+  private static final PrimitiveType FLOAT_TYPE = 
Types.optional(PrimitiveTypeName.FLOAT)
+      .columnOrder(ColumnOrder.typeDefined())
+      .named("test_float");
+  private static final PrimitiveType DOUBLE_TYPE = 
Types.optional(PrimitiveTypeName.DOUBLE)
+      .columnOrder(ColumnOrder.typeDefined())
+      .named("test_double");
   private static final PrimitiveType FLOAT16_TYPE = 
Types.optional(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
       .length(2)
       .as(LogicalTypeAnnotation.float16Type())
+      .columnOrder(ColumnOrder.typeDefined())
       .named("test_float16");
 
   private static final PrimitiveType FLOAT_IEEE754_TYPE = 
Types.optional(PrimitiveTypeName.FLOAT)
diff --git 
a/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilder.java
 
b/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilder.java
index 6f11f81fa..c8061afa4 100644
--- 
a/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilder.java
+++ 
b/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilder.java
@@ -69,6 +69,7 @@ import 
org.apache.parquet.filter2.predicate.Operators.IntColumn;
 import org.apache.parquet.filter2.predicate.Operators.LongColumn;
 import org.apache.parquet.filter2.predicate.UserDefinedPredicate;
 import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.ColumnOrder;
 import org.apache.parquet.schema.PrimitiveType;
 import org.apache.parquet.schema.Types;
 import org.junit.jupiter.api.Test;
@@ -1030,7 +1031,8 @@ public class TestColumnIndexBuilder {
 
   @Test
   public void testBuildDoubleZeroNaN() {
-    PrimitiveType type = Types.required(DOUBLE).named("test_double");
+    PrimitiveType type =
+        
Types.required(DOUBLE).columnOrder(ColumnOrder.typeDefined()).named("test_double");
     ColumnIndexBuilder builder = ColumnIndexBuilder.getBuilder(type, 
Integer.MAX_VALUE);
     StatsBuilder sb = new StatsBuilder();
     builder.add(sb.stats(type, -1.0, -0.0));
@@ -1183,7 +1185,8 @@ public class TestColumnIndexBuilder {
 
   @Test
   public void testBuildFloatZeroNaN() {
-    PrimitiveType type = Types.required(FLOAT).named("test_float");
+    PrimitiveType type =
+        
Types.required(FLOAT).columnOrder(ColumnOrder.typeDefined()).named("test_float");
     ColumnIndexBuilder builder = ColumnIndexBuilder.getBuilder(type, 
Integer.MAX_VALUE);
     StatsBuilder sb = new StatsBuilder();
     builder.add(sb.stats(type, -1.0f, -0.0f));
diff --git 
a/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilderNaN.java
 
b/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilderNaN.java
index 342e79b0b..90b6f90a3 100644
--- 
a/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilderNaN.java
+++ 
b/parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilderNaN.java
@@ -44,19 +44,22 @@ import org.junit.jupiter.api.Test;
  */
 public class TestColumnIndexBuilderNaN {
 
-  private static final PrimitiveType FLOAT_TYPE =
-      Types.required(PrimitiveTypeName.FLOAT).named("test_float");
+  private static final PrimitiveType FLOAT_TYPE = 
Types.required(PrimitiveTypeName.FLOAT)
+      .columnOrder(ColumnOrder.typeDefined())
+      .named("test_float");
   private static final PrimitiveType FLOAT_IEEE754_TYPE = 
Types.required(PrimitiveTypeName.FLOAT)
       .columnOrder(ColumnOrder.ieee754TotalOrder())
       .named("test_float_ieee754");
-  private static final PrimitiveType DOUBLE_TYPE =
-      Types.required(PrimitiveTypeName.DOUBLE).named("test_double");
+  private static final PrimitiveType DOUBLE_TYPE = 
Types.required(PrimitiveTypeName.DOUBLE)
+      .columnOrder(ColumnOrder.typeDefined())
+      .named("test_double");
   private static final PrimitiveType DOUBLE_IEEE754_TYPE = 
Types.required(PrimitiveTypeName.DOUBLE)
       .columnOrder(ColumnOrder.ieee754TotalOrder())
       .named("test_double_ieee754");
   private static final PrimitiveType FLOAT16_TYPE = 
Types.required(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
       .length(2)
       .as(LogicalTypeAnnotation.float16Type())
+      .columnOrder(ColumnOrder.typeDefined())
       .named("test_float16");
   private static final PrimitiveType FLOAT16_IEEE754_TYPE = 
Types.required(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
       .length(2)
diff --git 
a/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java 
b/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java
index 5f6e46339..d8fae9ac1 100644
--- 
a/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java
+++ 
b/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java
@@ -199,10 +199,76 @@ public class TestMessageType {
                 Types.optional(INT96).named("b"),
                 Types.optional(BINARY).named("c"))
             .named("root"));
-    assertThatThrownBy(() -> m1.union(m3))
-        .isInstanceOf(IncompatibleSchemaModificationException.class)
-        .hasMessage(
-            "can not merge type optional binary a with column order 
TYPE_DEFINED_ORDER into optional binary a with column order UNDEFINED");
+    // Merging columns that differ only in column order reconciles to 
UNDEFINED rather than failing:
+    // m1's "a" is UNDEFINED and m3's "a" is TYPE_DEFINED_ORDER, so the merged 
"a" stays UNDEFINED
+    // (schema equality includes column order, so equality with m1 asserts the 
reconciled order).
+    assertThat(m1.union(m3)).isEqualTo(m1);
+  }
+
+  @Test
+  public void testColumnOrderTextRoundTrip() {
+    // A non-default column order must survive toString() -> 
parseMessageType() so that schemas
+    // serialized through the text representation (e.g. by GroupWriteSupport) 
keep it.
+    MessageType schema = Types.buildMessage()
+        .required(PrimitiveTypeName.FLOAT)
+        .columnOrder(ColumnOrder.typeDefined())
+        .named("float_typedef")
+        .required(PrimitiveTypeName.DOUBLE)
+        .columnOrder(ColumnOrder.ieee754TotalOrder())
+        .named("double_ieee754")
+        .required(PrimitiveTypeName.INT32)
+        .named("int_default")
+        .named("msg");
+
+    assertThat(schema.getType("float_typedef").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+    MessageType roundTripped = 
MessageTypeParser.parseMessageType(schema.toString());
+    assertThat(roundTripped).isEqualTo(schema);
+    
assertThat(roundTripped.getType("float_typedef").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+    
assertThat(roundTripped.getType("double_ieee754").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+    
assertThat(roundTripped.getType("int_default").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+
+    // A column left at its default emits no columnorder(...) token.
+    assertThat(schema.toString()).doesNotContain("int_default columnorder");
+  }
+
+  @Test
+  public void testUnknownColumnOrderParsesAsUndefined() {
+    // A column order this version does not recognize (e.g. written by a newer 
API) degrades to
+    // UNDEFINED rather than failing the whole parse.
+    MessageType schema =
+        MessageTypeParser.parseMessageType("message msg { required binary a 
columnorder(SOME_FUTURE_ORDER); }");
+    
assertThat(schema.getType("a").asPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.undefined());
+  }
+
+  @Test
+  public void testMergeMixedFloatingColumnOrder() {
+    // A float column written post-upgrade (IEEE 754 total order) and the same 
column read from a
+    // legacy footer (type-defined) must merge rather than throw -- e.g. when 
aggregating footers
+    // over a directory that spans the upgrade. The reconciled order is 
UNDEFINED; per-file
+    // statistics are still read under each file's own column order.
+    MessageType newFile = Types.buildMessage()
+        .required(PrimitiveTypeName.FLOAT)
+        .columnOrder(ColumnOrder.ieee754TotalOrder())
+        .named("f")
+        .named("root");
+    MessageType legacyFile = Types.buildMessage()
+        .required(PrimitiveTypeName.FLOAT)
+        .columnOrder(ColumnOrder.typeDefined())
+        .named("f")
+        .named("root");
+
+    MessageType merged = newFile.union(legacyFile);
+    
assertThat(merged.getType("f").asPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.undefined());
+    // Merge is symmetric.
+    
assertThat(legacyFile.union(newFile).getType("f").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.undefined());
+    // Same order on both sides is preserved (no spurious downgrade to 
UNDEFINED).
+    
assertThat(newFile.union(newFile).getType("f").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.ieee754TotalOrder());
   }
 
   @Test
diff --git 
a/parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java
 
b/parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java
index 465516e48..473a15710 100644
--- 
a/parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java
+++ 
b/parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java
@@ -2061,6 +2061,13 @@ public class ParquetMetadataConverter {
             columnOrder = org.apache.parquet.schema.ColumnOrder.undefined();
           }
           primitiveBuilder.columnOrder(columnOrder);
+        } else if (schemaElement.type == Type.FLOAT
+            || schemaElement.type == Type.DOUBLE
+            || (schemaElement.isSetLogicalType() && 
schemaElement.logicalType.isSetFLOAT16())) {
+          // A footer without column orders predates IEEE_754_TOTAL_ORDER, so 
a floating-point column
+          // here must not inherit the (IEEE 754 total order) 
construction-time default: its stats, if
+          // any, were written under the legacy type-defined order and must be 
read under it.
+          
primitiveBuilder.columnOrder(org.apache.parquet.schema.ColumnOrder.typeDefined());
         }
         childBuilder = primitiveBuilder;
       } else {
diff --git 
a/parquet-hadoop/src/test/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilterTest.java
 
b/parquet-hadoop/src/test/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilterTest.java
index 8272042f6..d50306caf 100644
--- 
a/parquet-hadoop/src/test/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilterTest.java
+++ 
b/parquet-hadoop/src/test/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilterTest.java
@@ -706,15 +706,20 @@ public class DictionaryFilterTest {
     return List.of(
         nanColumn(
             "double_nan_field",
-            
Types.required(PrimitiveTypeName.DOUBLE).named("double_nan_field")),
+            Types.required(PrimitiveTypeName.DOUBLE)
+                .columnOrder(ColumnOrder.typeDefined())
+                .named("double_nan_field")),
         nanColumn(
             "float_nan_field",
-            Types.required(PrimitiveTypeName.FLOAT).named("float_nan_field")),
+            Types.required(PrimitiveTypeName.FLOAT)
+                .columnOrder(ColumnOrder.typeDefined())
+                .named("float_nan_field")),
         nanColumn(
             "float16_nan_field",
             Types.required(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
                 .length(2)
                 .as(LogicalTypeAnnotation.float16Type())
+                .columnOrder(ColumnOrder.typeDefined())
                 .named("float16_nan_field")));
   }
 
diff --git 
a/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java
 
b/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java
index 4d361d6aa..576a16979 100644
--- 
a/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java
+++ 
b/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java
@@ -2133,6 +2133,72 @@ public class TestParquetMetadataConverter {
         .isEqualTo(ColumnOrder.ieee754TotalOrder());
   }
 
+  @Test
+  public void testFloatingPointColumnsDefaultToIeee754TotalOrder() throws 
IOException {
+    MessageType schema = parseMessageType("message test {"
+        + "  required float float_col;"
+        + "  required double double_col;"
+        + "  required fixed_len_byte_array(2) float16_col (FLOAT16);"
+        + "  required int32 int_col;"
+        + "}");
+
+    org.apache.parquet.hadoop.metadata.FileMetaData fileMetaData =
+        new org.apache.parquet.hadoop.metadata.FileMetaData(schema, new 
HashMap<String, String>(), null);
+    ParquetMetadata metadata = new ParquetMetadata(fileMetaData, new 
ArrayList<BlockMetaData>());
+    ParquetMetadataConverter converter = new ParquetMetadataConverter();
+    FileMetaData formatMetadata = converter.toParquetMetadata(1, metadata);
+
+    // Floating-point columns serialize the new order; the int column keeps 
type-defined order.
+    List<org.apache.parquet.format.ColumnOrder> columnOrders = 
formatMetadata.getColumn_orders();
+    assertThat(columnOrders).hasSize(4);
+    assertThat(columnOrders.get(0).isSetIEEE_754_TOTAL_ORDER()).isTrue();
+    assertThat(columnOrders.get(1).isSetIEEE_754_TOTAL_ORDER()).isTrue();
+    assertThat(columnOrders.get(2).isSetIEEE_754_TOTAL_ORDER()).isTrue();
+    assertThat(columnOrders.get(3).isSetTYPE_ORDER()).isTrue();
+
+    MessageType resultSchema =
+        
converter.fromParquetMetadata(formatMetadata).getFileMetaData().getSchema();
+    
assertThat(resultSchema.getType("float_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+    
assertThat(resultSchema.getType("double_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+    
assertThat(resultSchema.getType("float16_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+    assertThat(resultSchema.getType("int_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+  }
+
+  @Test
+  public void testUndefinedFloatingPointColumnOrderReadsAsTypeDefined() throws 
IOException {
+    // A footer without column orders predates IEEE_754_TOTAL_ORDER: 
floating-point columns must be
+    // read back as type-defined order (the pre-existing default) so that 
legacy stats are not
+    // reinterpreted under IEEE 754 total order.
+    MessageType schema = parseMessageType("message test {"
+        + "  required float float_col;"
+        + "  required double double_col;"
+        + "  required fixed_len_byte_array(2) float16_col (FLOAT16);"
+        + "}");
+
+    org.apache.parquet.hadoop.metadata.FileMetaData fileMetaData =
+        new org.apache.parquet.hadoop.metadata.FileMetaData(schema, new 
HashMap<String, String>(), null);
+    ParquetMetadata metadata = new ParquetMetadata(fileMetaData, new 
ArrayList<BlockMetaData>());
+    ParquetMetadataConverter converter = new ParquetMetadataConverter();
+    FileMetaData formatMetadata = converter.toParquetMetadata(1, metadata);
+
+    // Simulate a legacy footer that carries no column orders at all.
+    formatMetadata.unsetColumn_orders();
+    assertThat(formatMetadata.isSetColumn_orders()).isFalse();
+
+    MessageType resultSchema =
+        
converter.fromParquetMetadata(formatMetadata).getFileMetaData().getSchema();
+    
assertThat(resultSchema.getType("float_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+    
assertThat(resultSchema.getType("double_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+    
assertThat(resultSchema.getType("float16_col").asPrimitiveType().columnOrder())
+        .isEqualTo(ColumnOrder.typeDefined());
+  }
+
   @Test
   public void testNestedColumnOrdersUseLeafOrder() throws IOException {
     MessageType schema = Types.buildMessage()
@@ -2159,7 +2225,9 @@ public class TestParquetMetadataConverter {
     List<ColumnDescriptor> columns = resultSchema.getColumns();
     assertThat(columns).hasSize(3);
     
assertThat(columns.get(0).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.ieee754TotalOrder());
-    
assertThat(columns.get(1).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.typeDefined());
+    // Column "b" is a DOUBLE built without an explicit column order, so it 
picks up the
+    // floating-point default of IEEE 754 total order.
+    
assertThat(columns.get(1).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.ieee754TotalOrder());
     
assertThat(columns.get(2).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.ieee754TotalOrder());
   }
 
diff --git 
a/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/FloatingPointNanInteropFileGenerator.java
 
b/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/FloatingPointNanInteropFileGenerator.java
index df4b8ce88..ff38bb1ad 100644
--- 
a/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/FloatingPointNanInteropFileGenerator.java
+++ 
b/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/FloatingPointNanInteropFileGenerator.java
@@ -71,11 +71,13 @@ public final class FloatingPointNanInteropFileGenerator {
       .columnOrder(ColumnOrder.ieee754TotalOrder())
       .named("float_ieee754")
       .required(FLOAT)
+      .columnOrder(ColumnOrder.typeDefined())
       .named("float_typedef")
       .required(DOUBLE)
       .columnOrder(ColumnOrder.ieee754TotalOrder())
       .named("double_ieee754")
       .required(DOUBLE)
+      .columnOrder(ColumnOrder.typeDefined())
       .named("double_typedef")
       .required(FIXED_LEN_BYTE_ARRAY)
       .length(2)
@@ -85,6 +87,7 @@ public final class FloatingPointNanInteropFileGenerator {
       .required(FIXED_LEN_BYTE_ARRAY)
       .length(2)
       .as(float16Type())
+      .columnOrder(ColumnOrder.typeDefined())
       .named("float16_typedef")
       .named("msg");
 
diff --git 
a/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16ReadWriteRoundTrip.java
 
b/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16ReadWriteRoundTrip.java
index e5b968c6a..a7dcea86a 100644
--- 
a/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16ReadWriteRoundTrip.java
+++ 
b/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16ReadWriteRoundTrip.java
@@ -39,6 +39,7 @@ import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
 import org.apache.parquet.hadoop.util.HadoopInputFile;
 import org.apache.parquet.internal.column.columnindex.ColumnIndex;
 import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.ColumnOrder;
 import org.apache.parquet.schema.MessageType;
 import org.apache.parquet.schema.Types;
 import org.junit.jupiter.api.Test;
@@ -142,6 +143,7 @@ public class TestFloat16ReadWriteRoundTrip {
           .required(FIXED_LEN_BYTE_ARRAY)
           .as(float16Type())
           .length(2)
+          .columnOrder(ColumnOrder.typeDefined())
           .named("col_float16")
           .named("msg");
 
@@ -178,6 +180,7 @@ public class TestFloat16ReadWriteRoundTrip {
         .required(FIXED_LEN_BYTE_ARRAY)
         .as(float16Type())
         .length(2)
+        .columnOrder(ColumnOrder.typeDefined())
         .named("col_float16")
         .named("msg");
 
diff --git 
a/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16Statistics.java
 
b/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16Statistics.java
index a413c6233..fe952d9e3 100644
--- 
a/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16Statistics.java
+++ 
b/parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestFloat16Statistics.java
@@ -37,6 +37,7 @@ import org.apache.parquet.hadoop.example.GroupWriteSupport;
 import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
 import org.apache.parquet.hadoop.util.HadoopInputFile;
 import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.ColumnOrder;
 import org.apache.parquet.schema.Float16;
 import org.apache.parquet.schema.MessageType;
 import org.apache.parquet.schema.Types;
@@ -156,6 +157,7 @@ public class TestFloat16Statistics {
           .required(FIXED_LEN_BYTE_ARRAY)
           .as(float16Type())
           .length(2)
+          .columnOrder(ColumnOrder.typeDefined())
           .named("col_float16")
           .named("msg");
 
@@ -213,6 +215,7 @@ public class TestFloat16Statistics {
             .required(FIXED_LEN_BYTE_ARRAY)
             .as(float16Type())
             .length(2)
+            .columnOrder(ColumnOrder.typeDefined())
             .named("col_float16")
             .named("msg");
 

Reply via email to