remove flags

Author: divjotarora

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

@@ -38,9 +38,7 @@ public enum ColumnOrderName {
      */
     TYPE_DEFINED_ORDER,
     /**
-     * Chronological order for INT96 timestamps: values are compared by the Julian day (the last 4
-     * bytes, as a little-endian signed int32), then by the nanoseconds within the day (the first 8
-     * bytes, as a little-endian signed int64). Only supported for the INT96 physical type.
+     * Chronological order for INT96 timestamps. Only valid for the INT96 physical type.
      */
     INT96_TIMESTAMP_ORDER
   }

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

@@ -566,9 +566,15 @@ public PrimitiveType(
     this.decimalMeta = decimalMeta;
 
     if (columnOrder == null) {
-      columnOrder = primitive == PrimitiveTypeName.INT96 || originalType == OriginalType.INTERVAL
-          ? ColumnOrder.undefined()
-          : ColumnOrder.typeDefined();
+      if (primitive == PrimitiveTypeName.INT96) {
+        // A plain INT96 is the legacy timestamp encoding; default it to the chronological order.
+        // An annotated INT96 carries other semantics, so leave its order undefined.
+        columnOrder = originalType == null ? ColumnOrder.int96TimestampOrder() : ColumnOrder.undefined();
+      } else if (originalType == OriginalType.INTERVAL) {
+        columnOrder = ColumnOrder.undefined();
+      } else {
+        columnOrder = ColumnOrder.typeDefined();
+      }
     }
     this.columnOrder = requireValidColumnOrder(columnOrder);
   }
@@ -611,10 +617,18 @@ public PrimitiveType(
     }
 
     if (columnOrder == null) {
-      columnOrder = primitive == PrimitiveTypeName.INT96
-              || logicalTypeAnnotation instanceof LogicalTypeAnnotation.IntervalLogicalTypeAnnotation
-          ? ColumnOrder.undefined()
-          : ColumnOrder.typeDefined();
+      if (primitive == PrimitiveTypeName.INT96) {
+        // A plain INT96 is the legacy timestamp encoding; default it to the chronological order.
+        // An annotated INT96 carries other semantics, so leave its order undefined.
+        columnOrder = logicalTypeAnnotation == null ?
+          ColumnOrder.int96TimestampOrder() :
+          ColumnOrder.undefined();
+      } else if (
+        logicalTypeAnnotation instanceof LogicalTypeAnnotation.IntervalLogicalTypeAnnotation) {
+        columnOrder = ColumnOrder.undefined();
+      } else {
+        columnOrder = ColumnOrder.typeDefined();
+      }
     }
     this.columnOrder = requireValidColumnOrder(columnOrder);
   }
@@ -661,15 +675,6 @@ public PrimitiveType withLogicalTypeAnnotation(LogicalTypeAnnotation logicalType
     return new PrimitiveType(getRepetition(), primitive, length, getName(), logicalType, getId());
   }
 
-  /**
-   * @param columnOrder the column order
-   * @return a new PrimitiveType with the same fields and the given column order
-   */
-  public PrimitiveType withColumnOrder(ColumnOrder columnOrder) {
-    return new PrimitiveType(
-        getRepetition(), primitive, length, getName(), getLogicalTypeAnnotation(), getId(), columnOrder);
-  }
-
   /**
    * @return the primitive type
    */

parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestBinaryTruncator.java

@@ -93,7 +93,16 @@ public void testContractNonStringTypes() {
     testTruncator(
         Types.required(FIXED_LEN_BYTE_ARRAY).length(12).as(INTERVAL).named("test_fixed_interval"), false);
     testTruncator(Types.required(BINARY).as(DECIMAL).precision(10).scale(2).named("test_binary_decimal"), false);
-    testTruncator(Types.required(INT96).named("test_int96"), false);
+
+    // INT96 has a fixed 12-byte width and a chronological comparator (so it is excluded from the
+    // variable-length checks above, like FLOAT16). Its truncator is a no-op: verify it returns the
+    // value unchanged regardless of the requested length.
+    BinaryTruncator int96Truncator = BinaryTruncator.getTruncator(
+      Types.required(INT96).named("test_int96"));
+    Binary int96Value = Binary.fromConstantByteArray(
+      new byte[] {0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4});
+    assertSame(int96Value, int96Truncator.truncateMin(int96Value, 4));
+    assertSame(int96Value, int96Truncator.truncateMax(int96Value, 4));
   }
 
   @Test

parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java

@@ -2043,12 +2043,17 @@ private void buildChildren(
                   || schemaElement.converted_type == ConvertedType.INTERVAL)) {
             columnOrder = org.apache.parquet.schema.ColumnOrder.undefined();
           }
-          // INT96_TIMESTAMP_ORDER is only valid for INT96 columns; ignore it anywhere else
+          // INT96_TIMESTAMP_ORDER is only valid for INT96 columns, ignore it anywhere else.
           if (columnOrder.getColumnOrderName() == ColumnOrderName.INT96_TIMESTAMP_ORDER
               && schemaElement.type != Type.INT96) {
             columnOrder = org.apache.parquet.schema.ColumnOrder.undefined();
           }
           primitiveBuilder.columnOrder(columnOrder);
+        } else if (schemaElement.type == Type.INT96) {
+          // A footer without column orders predates INT96_TIMESTAMP_ORDER, so an INT96 column here
+          // must not inherit the (chronological) construction-time default: its stats, if any, were
+          // written under the legacy order and must be ignored.
+          primitiveBuilder.columnOrder(org.apache.parquet.schema.ColumnOrder.undefined());
         }
         childBuilder = primitiveBuilder;
       } else {

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/InternalParquetRecordWriter.java

@@ -89,7 +89,7 @@ public InternalParquetRecordWriter(
       ParquetProperties props) {
     this.parquetFileWriter = parquetFileWriter;
     this.writeSupport = Objects.requireNonNull(writeSupport, "writeSupport cannot be null");
-    this.schema = ParquetFileWriter.applyInt96TimestampOrder(schema);
+    this.schema = schema;
     this.extraMetaData = extraMetaData;
     this.rowGroupSizeThreshold = rowGroupSize;
     this.rowGroupRecordCountThreshold = props.getRowGroupRowCountLimit();

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileWriter.java

@@ -92,12 +92,8 @@
 import org.apache.parquet.io.ParquetEncodingException;
 import org.apache.parquet.io.PositionOutputStream;
 import org.apache.parquet.io.SeekableInputStream;
-import org.apache.parquet.schema.ColumnOrder;
-import org.apache.parquet.schema.GroupType;
 import org.apache.parquet.schema.MessageType;
 import org.apache.parquet.schema.PrimitiveType;
-import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
-import org.apache.parquet.schema.Type;
 import org.apache.parquet.schema.TypeUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -439,7 +435,7 @@ public ParquetFileWriter(
       throws IOException {
     this(
         file,
-        applyInt96TimestampOrder(schema),
+        schema,
         mode,
         rowGroupSize,
         maxPaddingSize,
@@ -451,32 +447,6 @@ public ParquetFileWriter(
         props.getAllocator());
   }
 
-  /**
-   * Returns the schema with INT96 columns tagged with the INT96_TIMESTAMP_ORDER column order, so
-   * that statistics are accumulated with the chronological comparator and the proper column order
-   * is written to the footer.
-   */
-  static MessageType applyInt96TimestampOrder(MessageType schema) {
-    return new MessageType(schema.getName(), applyInt96TimestampOrder(schema.getFields()));
-  }
-
-  private static List<Type> applyInt96TimestampOrder(List<Type> fields) {
-    List<Type> result = new ArrayList<>(fields.size());
-    for (Type field : fields) {
-      if (field.isPrimitive()) {
-        PrimitiveType primitive = field.asPrimitiveType();
-        if (primitive.getPrimitiveTypeName() == PrimitiveTypeName.INT96) {
-          field = primitive.withColumnOrder(ColumnOrder.int96TimestampOrder());
-        }
-        result.add(field);
-      } else {
-        GroupType group = field.asGroupType();
-        result.add(group.withNewFields(applyInt96TimestampOrder(group.getFields())));
-      }
-    }
-    return result;
-  }
-
   @Deprecated
   public ParquetFileWriter(
       OutputFile file,

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

@@ -1170,17 +1170,14 @@ public void testMissingValuesFromStats() {
 
   @Test
   public void testSkippedV2Stats() {
+    // INTERVAL has an undefined column order, so its stats are skipped.
     testSkippedV2Stats(
         Types.optional(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
             .length(12)
             .as(OriginalType.INTERVAL)
             .named(""),
         new BigInteger("12345678"),
         new BigInteger("12345679"));
-    testSkippedV2Stats(
-        Types.optional(PrimitiveTypeName.INT96).named(""),
-        new BigInteger("-75687987"),
-        new BigInteger("45367657"));
   }
 
   private void testSkippedV2Stats(PrimitiveType type, Object min, Object max) {
@@ -1382,8 +1379,7 @@ public void testColumnOrders() throws IOException {
         + "        required binary key (UTF8);" // Key to be hacked to have unknown column order -> undefined
         + "        optional group list_col (LIST) {"
         + "          repeated group list {"
-        + "            optional int96 array_element;" // INT96 element with type defined column order ->
-        // undefined
+        + "            optional int96 array_element;" // plain INT96 element -> INT96_TIMESTAMP_ORDER
         + "          }"
         + "        }"
         + "    }"
@@ -1397,9 +1393,10 @@ public void testColumnOrders() throws IOException {
 
     List<org.apache.parquet.format.ColumnOrder> columnOrders = formatMetadata.getColumn_orders();
     assertEquals(3, columnOrders.size());
-    for (org.apache.parquet.format.ColumnOrder columnOrder : columnOrders) {
-      assertTrue(columnOrder.isSetTYPE_ORDER());
-    }
+    // binary_col and key get TYPE_ORDER, the INT96 array_element gets INT96_TIMESTAMP_ORDER.
+    assertTrue(columnOrders.get(0).isSetTYPE_ORDER());
+    assertTrue(columnOrders.get(1).isSetTYPE_ORDER());
+    assertTrue(columnOrders.get(2).isSetINT96_TIMESTAMP_ORDER());
 
     // Simulate that thrift got a union type that is not in the generated code
     // (when the file contains a not-yet-supported column order)
@@ -1412,7 +1409,7 @@ public void testColumnOrders() throws IOException {
     assertEquals(
         ColumnOrder.typeDefined(), columns.get(0).getPrimitiveType().columnOrder());
     assertEquals(ColumnOrder.undefined(), columns.get(1).getPrimitiveType().columnOrder());
-    assertEquals(ColumnOrder.undefined(), columns.get(2).getPrimitiveType().columnOrder());
+    assertEquals(ColumnOrder.int96TimestampOrder(), columns.get(2).getPrimitiveType().columnOrder());
   }
 
   @Test
@@ -1487,7 +1484,11 @@ public void testColumnIndexConversion() {
       assertNull(
           "Should ignore unsupported types",
           ParquetMetadataConverter.toParquetColumnIndex(
-              Types.required(PrimitiveTypeName.INT96).named("test_int96"), columnIndex));
+              Types.required(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
+                  .length(12)
+                  .as(OriginalType.INTERVAL)
+                  .named("test_interval"),
+              columnIndex));
       assertNull(
           "Should ignore unsupported types",
           ParquetMetadataConverter.fromParquetColumnIndex(

parquet-hadoop/src/test/java/org/apache/parquet/statistics/TestInt96TimestampStatistics.java

@@ -189,6 +189,8 @@ private static void assertStatsUsable(ColumnChunkMetaData column) {
 
   @Test
   public void testWriterEmitsInt96StatsAndColumnOrder() throws IOException {
+    // Values are written in non-chronological order; the writer must still produce the
+    // chronological min/max, not first/last or byte-wise extremes.
     File file = writeFile();
     FileMetaData rawFooter = readRawFooter(file);
     // schema[0] is the message root; column_orders are indexed by leaf order: ts=0, id=1
@@ -224,19 +226,6 @@ public void testWriterEmitsInt96StatsAndColumnOrder() throws IOException {
     }
   }
 
-  @Test
-  public void testStatsAccumulationUsesChronologicalOrder() throws IOException {
-    // Values are written in non-chronological order; the writer must still produce the
-    // chronological min/max, not first/last or byte-wise extremes.
-    File file = writeFile();
-    ParquetMetadata footer = readFooter(file, new Configuration());
-    byte[] minBytes = getColumn(footer, "ts").getStatistics().getMinBytes();
-    assertFalse(Binary.fromConstantByteArray(minBytes).equals(NEXT_DAY));
-    assertArrayEquals(EXPECTED_MIN.getBytes(), minBytes);
-    assertArrayEquals(EXPECTED_MAX.getBytes(),
-      getColumn(footer, "ts").getStatistics().getMaxBytes());
-  }
-
   @Test
   public void testReaderReadsStatsWrittenWithInt96TimestampOrder() throws IOException {
     File file = writeFile();
@@ -255,7 +244,7 @@ public void testReaderIgnoresInt96StatsWithTypeDefinedOrder() throws IOException
     File file = writeFile();
     FileMetaData rawFooter = readRawFooter(file);
     downgradeInt96ToTypeOrder(rawFooter);
-    File legacy = rewriteFooter(file, rawFooter, "legacy.parquet");
+    File legacy = rewriteFooter(file, rawFooter, "type-defined-orders.parquet");
 
     // Validate the data is still intact after rewriting the footer.
     try (
@@ -278,6 +267,22 @@ public void testReaderIgnoresInt96StatsWithTypeDefinedOrder() throws IOException
     assertTrue(getColumn(footer, "id").getStatistics().hasNonNullValue());
   }
 
+  @Test
+  public void testReaderIgnoresInt96StatsWhenColumnOrdersAbsent() throws IOException {
+    // Legacy layout: INT96 stats present but the footer omits column_orders entirely (predating the
+    // order). The reader must not infer INT96_TIMESTAMP_ORDER from the construction-time default
+    // and must therefore ignore the stats.
+    File file = writeFile();
+    FileMetaData rawFooter = readRawFooter(file);
+    rawFooter.unsetColumn_orders();
+    File legacy = rewriteFooter(file, rawFooter, "no-column-orders.parquet");
+
+    ParquetMetadata footer = readFooter(legacy, new Configuration());
+    assertEquals(ColumnOrder.undefined(),
+        footer.getFileMetaData().getSchema().getType("ts").asPrimitiveType().columnOrder());
+    assertStatsIgnored(getColumn(footer, "ts"));
+  }
+
   private static byte[] toArray(ByteBuffer buffer) {
     byte[] bytes = new byte[buffer.remaining()];
     buffer.duplicate().get(bytes);