thswlsqls opened a new pull request, #17269:
URL: https://github.com/apache/iceberg/pull/17269

   
   Closes #17268
   
   ## Summary
   
   `testMaxColumnsWithDefaultOverride` claims that setting the default metrics 
mode explicitly bypasses the max-inferred-column limit, so all 101 columns keep 
metrics. It never checked that.
   
   - Lower bounds were read from `dataFile.upperBounds()`, so `lowerBounds()` 
was never called.
   - The loop variable was unused: both assertions hardcoded `get(1)`, so only 
column 1 was checked, 101 times. The other 100 could lose metrics and the test 
would stay green.
   
   Fix: read lower bounds from `lowerBounds()`, assert both maps hold every 
column, and use the loop variable. IDs start at 1 because the table reassigns 
them, as in the sibling `testMaxColumns`. The size assertion mirrors 
`testMaxColumnsBounded`.
   
   Test-only; unchanged since #3959.
   
   ## Testing done
   
   `TestWriterMetrics` is abstract with no subclass in `data/`, so 
`:iceberg-data:test` never runs it. Verified through two of its six subclasses, 
covering ORC and Parquet:
   
   - `-DflinkVersions=1.20 :iceberg-flink:iceberg-flink-1.20:test --tests 
"*TestFlinkWriterMetrics"` — 12 tests, 0 failures.
   - `:iceberg-spark:iceberg-spark-4.1_2.13:test --tests 
"*TestSparkWriterMetrics"` — 12 tests, 0 failures.
   - `:iceberg-data:spotlessCheck` — passed.
   
   Suppressing the bypass in `MetricsConfig.from()` makes the new assertions 
fail (`Expected size: 101 but was: 100`); the old ones passed. That edit was 
reverted.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to