gkalashyan-akv commented on code in PR #6: URL: https://github.com/apache/iceberg-verification/pull/6#discussion_r4183651994
########## reference-tables/README.md: ########## @@ -0,0 +1,142 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. + --> + +# Reference tables + +This directory hosts whole Iceberg tables reference fixtures. +Surfaces reference a table by path and hold their own expected +values, so one table can support several surfaces without being copied. + +Tables are grouped by format version. + +## Size + +A table should stay under 100 KB. The three here are 16 KB to 28 KB. + +Size tracks commit count, and almost nothing else. A commit costs roughly 12 to +14 KB, over half of it the manifest and manifest list it produces. Rows are close +to free: a Parquet file holding five rows is under 1 KB and is nearly all footer. +A deletion vector is 385 bytes. A v3 table runs slightly heavier per commit than +v1 or v2, because a v3 manifest entry carries the deletion vector fields whether +or not a vector exists. + +So the limit is about seven commits. A table that needs more should say why in +its pull request. + +## Paths + +Prior to v4 the spec requires every path field to be fully qualified, so each +table carries an absolute prefix in `location`, in every `manifest-list`, and in +every `file_path`. That prefix is the sentinel root: + +``` +file:///tmp/iceberg-verification/reference-tables/<format-version>/<table-name> +``` + +A consumer resolves it one of two ways. Symlinking `/tmp/iceberg-verification` at +the checkout leaves every committed byte untouched. Substituting the prefix while +loading works where symlinks do not, notably on Windows. Both are valid, because Review Comment: Following up on [the earlier discussion](https://github.com/apache/iceberg-verification/pull/6#issuecomment-5737120640): the prefix substitution path still appears in the current revision. Would prefix-rewriting actually work end to end? My understanding is that scanning would still fail on the absolute `manifest_path` inside the manifest-list Avro. Could we require a scan step in the setup verification rather than just opening metadata? That would confirm the referenced artifacts are actually accessible. ########## reference-tables/index.json: ########## @@ -0,0 +1,28 @@ +{ + "tables": [ Review Comment: Could we add a check that follows each index entry through every retained snapshot, manifest list, manifest, and data file, verifying that live references exist, recorded file lengths match, and all snapshots remain readable? Intentionally pruned files in `metadata-log` should be excluded. Without automation, there is no ongoing guarantee that a future change does not silently break an older snapshot's artifact chain. ########## reference-tables/index.json: ########## @@ -0,0 +1,28 @@ +{ + "tables": [ + { + "name": "v2/simple-append-parquet-data", + "format-version": 2, + "written-by": "Iceberg Java 1.11.0", + "data-file-format": "parquet", + "metadata": "v2/simple-append-parquet-data/metadata/v3.metadata.json", + "description": "Flat schema of long, string and timestamptz. Two appends, four rows, one null." + }, + { + "name": "v2/nested-types-parquet-data", + "format-version": 2, + "written-by": "Iceberg Java 1.11.0", + "data-file-format": "parquet", + "metadata": "v2/nested-types-parquet-data/metadata/v2.metadata.json", + "description": "Struct, list and map, so every nested id space appears. One snapshot, two rows, one of them entirely null." Review Comment: The nested fixture currently contains populated containers and null containers, but no empty containers, null list elements, or null map values. Could we include those cases? The schema explicitly permits null elements and values, and these exercise different decoding behavior from a null list or map. ########## reference-tables/README.md: ########## @@ -0,0 +1,142 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. + --> + +# Reference tables + +This directory hosts whole Iceberg tables reference fixtures. +Surfaces reference a table by path and hold their own expected +values, so one table can support several surfaces without being copied. Review Comment: Keeping shared tables separate from surface-specific expectations makes sense. Could we link a companion surface that consumes one of these tables and defines exact expected values and comparison rules? Successful loading and scanning establish usability, but consumers still need independent expectations to establish conformance. -- 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]
