github-actions[bot] commented on code in PR #69031:
URL: https://github.com/apache/doris/pull/69031#discussion_r4269245149
##########
fe/pom.xml:
##########
@@ -360,7 +362,7 @@ under the License.
<!-- ATTN: avro version must be consistent with Iceberg version -->
<!-- Please modify iceberg.version and avro.version together,
you can find avro version info in iceberg mvn repository -->
- <iceberg.version>1.11.0</iceberg.version>
+ <iceberg.version>1.12.0</iceberg.version>
Review Comment:
[P2] Preserve metadata-task compatibility during rolling upgrades
With an FE still on Iceberg 1.11 and an upgraded BE on 1.12, `SELECT
snapshot_id FROM t$snapshots` is still scheduled as a normal JNI scan. Its real
`StaticDataTask` contains a non-null `GenericDataFile`/`BaseFile`, whose
serialized form/UID changed in 1.12, so `IcebergSysTableJniScanner` fails in
its constructor; the reverse upgrade order also fails. The Schema-only shim and
new exception message do not make this graph readable, and scalar system-table
scans have no version negotiation or compatible-backend routing. Preserve a
stable task representation/translate the actual metadata graph across the
supported upgrade pair, or negotiate compatible execution, and cover a real
non-null-metadata task in both directions (the existing old fixture has null
metadataFile).
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergManifestEntryKey.java:
##########
@@ -31,18 +33,25 @@
* cached payload, and a table-level invalidation (REFRESH TABLE)
intentionally does NOT drop it (legacy
* {@code testInvalidateTableKeepsManifestCache} parity). Runtime loader
context (the manifest/table instances)
* must not be stored here.
+ *
+ * <p>The key also carries the partition type the manifest is decoded with:
the reader converts partition values to
+ * the bound spec's types, and after a type promotion (e.g. INT to BIGINT) a
scan pinned to the old schema and a scan
+ * on the current one bind the same manifest to different types. Each binding
needs its own decoded copy.
*/
public class IcebergManifestEntryKey {
private final String manifestPath;
private final ManifestContent content;
+ private final Types.StructType partitionType;
Review Comment:
[P2] Include the bound partition type in cache admission weight
With manifest caching and a byte limit enabled, a historical snapshot can
contain a delete manifest with only DELETED entries (Iceberg retains manifests
authored by a remove-deletes commit). The reader then yields an empty
ManifestCacheValue, so its value estimate cannot reach this newly retained
StructType; estimateManifestKey still counts only shallow key size and the
path. specsFor(scan) creates fresh bound specs for historical scans, so cold
reads of distinct such manifests can retain separate partition-field/type
graphs outside both the manifest and catalog max-weight budgets, especially for
wide partition specs. Include the key's retained partition-type graph in
admission accounting and cover an empty delete-manifest value.
--
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]
