GH-3835: Preserve legacy float column order by default

parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java

@@ -587,7 +587,7 @@ public PrimitiveType(
     this.decimalMeta = decimalMeta;
 
     if (columnOrder == null) {
-      columnOrder = defaultColumnOrder(primitive, originalType, getLogicalTypeAnnotation());
+      columnOrder = defaultColumnOrder(primitive, originalType);
     } else if (columnOrder.getColumnOrderName() == ColumnOrderName.IEEE_754_TOTAL_ORDER) {
       Preconditions.checkArgument(
           primitive == PrimitiveTypeName.FLOAT || primitive == PrimitiveTypeName.DOUBLE,
@@ -636,7 +636,7 @@ public PrimitiveType(
     }
 
     if (columnOrder == null) {
-      columnOrder = defaultColumnOrder(primitive, getOriginalType(), logicalTypeAnnotation);
+      columnOrder = defaultColumnOrder(primitive, getOriginalType());
     } else if (columnOrder.getColumnOrderName() == ColumnOrderName.IEEE_754_TOTAL_ORDER) {
       Preconditions.checkArgument(
           primitive == PrimitiveTypeName.FLOAT
@@ -654,23 +654,15 @@ public PrimitiveType(
 
   /**
    * 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.
+   * ordering, so they default to undefined. Other types, including floating-point types, retain the
+   * type-defined order by default for compatibility with readers that predate IEEE 754 total order.
+   * Floating-point columns can opt in to IEEE 754 total order explicitly.
    */
-  private static ColumnOrder defaultColumnOrder(
-      PrimitiveTypeName primitive, OriginalType originalType, LogicalTypeAnnotation logicalTypeAnnotation) {
+  private static ColumnOrder defaultColumnOrder(PrimitiveTypeName primitive, OriginalType originalType) {
     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();
+    return ColumnOrder.typeDefined();
   }
 
   private ColumnOrder requireValidColumnOrder(ColumnOrder columnOrder) {
@@ -775,7 +767,7 @@ public void writeToStringBuilder(StringBuilder sb, String indent) {
     }
     // 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()))) {
+    if (!columnOrder.equals(defaultColumnOrder(primitive, getOriginalType()))) {
       sb.append(" ").append(COLUMN_ORDER_KEYWORD).append("(");
       sb.append(columnOrder.getColumnOrderName().name());
       sb.append(")");

parquet-column/src/main/java/org/apache/parquet/schema/Types.java

@@ -565,11 +565,10 @@ public THIS scale(int scale) {
     /**
      * Adds the column order for the primitive type.
      * <p>
-     * 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
+     * In case of not set the default column order is {@link ColumnOrderName#TYPE_DEFINED_ORDER}, except that the type
      * {@link PrimitiveTypeName#INT96} and the types annotated by {@link OriginalType#INTERVAL} default to
-     * {@link ColumnOrderName#UNDEFINED}.
+     * {@link ColumnOrderName#UNDEFINED}. Floating-point types can opt in to
+     * {@link ColumnOrderName#IEEE_754_TOTAL_ORDER} explicitly.
      *
      * @param columnOrder the column order for the primitive type
      * @return this builder for method chaining

parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java

@@ -2178,7 +2178,7 @@ public void testIEEE754TotalOrderColumnOrder() throws IOException {
   }
 
   @Test
-  public void testFloatingPointColumnsDefaultToIeee754TotalOrder() throws IOException {
+  public void testFloatingPointColumnsDefaultToTypeDefinedOrder() throws IOException {
     MessageType schema = parseMessageType("message test {"
         + "  required float float_col;"
         + "  required double double_col;"
@@ -2192,22 +2192,23 @@ public void testFloatingPointColumnsDefaultToIeee754TotalOrder() throws IOExcept
     ParquetMetadataConverter converter = new ParquetMetadataConverter();
     FileMetaData formatMetadata = converter.toParquetMetadata(1, metadata);
 
-    // Floating-point columns serialize the new order; the int column keeps type-defined order.
+    // Preserve the pre-1.18 default for interoperability with older readers. Callers can opt in
+    // to IEEE 754 total order explicitly on the schema.
     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(0).isSetTYPE_ORDER()).isTrue();
+    assertThat(columnOrders.get(1).isSetTYPE_ORDER()).isTrue();
+    assertThat(columnOrders.get(2).isSetTYPE_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());
+        .isEqualTo(ColumnOrder.typeDefined());
     assertThat(resultSchema.getType("double_col").asPrimitiveType().columnOrder())
-        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+        .isEqualTo(ColumnOrder.typeDefined());
     assertThat(resultSchema.getType("float16_col").asPrimitiveType().columnOrder())
-        .isEqualTo(ColumnOrder.ieee754TotalOrder());
+        .isEqualTo(ColumnOrder.typeDefined());
     assertThat(resultSchema.getType("int_col").asPrimitiveType().columnOrder())
         .isEqualTo(ColumnOrder.typeDefined());
   }
@@ -2269,9 +2270,9 @@ public void testNestedColumnOrdersUseLeafOrder() throws IOException {
     List<ColumnDescriptor> columns = resultSchema.getColumns();
     assertThat(columns).hasSize(3);
     assertThat(columns.get(0).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.ieee754TotalOrder());
-    // 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());
+    // Column "b" is a DOUBLE built without an explicit column order, so it retains type-defined
+    // order for compatibility with readers that predate IEEE 754 total order.
+    assertThat(columns.get(1).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.typeDefined());
     assertThat(columns.get(2).getPrimitiveType().columnOrder()).isEqualTo(ColumnOrder.ieee754TotalOrder());
   }