[I] Data: TestWriterMetrics.testMaxColumnsWithDefaultOverride does not verify bounds [iceberg]

["thswlsqls (via GitHub)" <gi...@apache.org>]

thswlsqls opened a new issue, #17268:
URL: https://github.com/apache/iceberg/issues/17268

   **Apache Iceberg version**
   
   main @ 5f3d3c560
   
   **Query engine**
   
   None — engine-agnostic. Defined in `data/`, run by the Flink and Spark implementations of `TestWriterMetrics`.
   
   **Please describe the bug**
   
   `TestWriterMetrics.testMaxColumnsWithDefaultOverride()` (`data/src/test/java/org/apache/iceberg/io/TestWriterMetrics.java` line 344-352) does not verify what its name and comment describe: that setting `write.metadata.metrics.default` explicitly bypasses the max-inferred-column limit, so all 101 columns keep metrics.
   
   - Line 346 reads lower bounds from `dataFile.upperBounds()`. `dataFile.lowerBounds()` is never called, so lower bounds go unchecked.
   - The loop variable at line 347 is unused; both assertions hardcode `get(1)`. Only column 1 is checked, 101 times.
   
   The test stays green even if the other 100 columns lose their metrics — the exact regression it exists to catch.
   
   **Steps to reproduce**
   
   Suppress the `configuredDefault != null` bypass in `MetricsConfig.from()` (`core/src/main/java/org/apache/iceberg/MetricsConfig.java` line 265) so `limitFieldIds` truncates to 100 columns.
   
   Expected: the test fails. Actual: it passes on both ORC and Parquet.
   
   **Additional context**
   
   Sibling tests in the same file show the intended form: `testMaxColumnsBounded` (line 307) asserts the bounds map size, and `testMaxColumns` (line 241-269) uses its loop variable to check `lowerBounds()` and `upperBounds()` separately.
   
   Introduced in #3959 (2022-02); the assertions have had this shape since.
   
   


-- 
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: issues-unsubscribe@iceberg.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [I] Data: TestWriterMetrics.testMaxColumnsWithDefaultOverride does not verify bounds [iceberg]

["binarybrain-009 (via GitHub)" <gi...@apache.org>]

binarybrain-009 commented on issue #17268:
URL: https://github.com/apache/iceberg/issues/17268#issuecomment-6499234815

   #17269 by @thswlsqls was moved to draft in July, closed by the stale bot in August without a review, and its branch was deleted in September. @thswlsqls, if you'd like to continue it, I'll step aside; otherwise I'd like to pick this up and credit your work. The plan is to read the lower bounds from `lowerBounds()` and check both bounds of every column through the loop variable, so the test fails when columns past the max-inferred limit lose their metrics.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org