Fail to execute Iceberg time travel query

[xiacongling]

After update Trino version to 392, queries failed for time travel queries.

trino> SELECT * FROM iceberg.meta.jobs for version as of 658617487758551825 limit 1;
Query 20220811_091143_00000_5rmtv failed: Internal error

Stacktrace:

java.lang.NullPointerException: undefined
	at org.apache.iceberg.SchemaParser.toJson(SchemaParser.java:172)
	at org.apache.iceberg.SchemaParser.toJson(SchemaParser.java:162)
	at io.trino.plugin.iceberg.IcebergMetadata.getTableHandle(IcebergMetadata.java:341)
	at io.trino.plugin.iceberg.IcebergMetadata.getTableHandle(IcebergMetadata.java:228)
	at io.trino.plugin.base.classloader.ClassLoaderSafeConnectorMetadata.getTableHandle(ClassLoaderSafeConnectorMetadata.java:1006)
	at io.trino.metadata.MetadataManager.lambda$getTableHandle$4(MetadataManager.java:268)
	at java.base/java.util.Optional.flatMap(Optional.java:294)
	at io.trino.metadata.MetadataManager.getTableHandle(MetadataManager.java:262)
	at io.trino.metadata.MetadataManager.getRedirectionAwareTableHandle(MetadataManager.java:1458)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.getTableHandle(StatementAnalyzer.java:4610)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitTable(StatementAnalyzer.java:1783)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitTable(StatementAnalyzer.java:447)
	at io.trino.sql.tree.Table.accept(Table.java:60)
	at io.trino.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.process(StatementAnalyzer.java:464)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.analyzeFrom(StatementAnalyzer.java:3732)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitQuerySpecification(StatementAnalyzer.java:2420)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitQuerySpecification(StatementAnalyzer.java:447)
	at io.trino.sql.tree.QuerySpecification.accept(QuerySpecification.java:155)
	at io.trino.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.process(StatementAnalyzer.java:464)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.process(StatementAnalyzer.java:472)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitQuery(StatementAnalyzer.java:1389)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.visitQuery(StatementAnalyzer.java:447)
	at io.trino.sql.tree.Query.accept(Query.java:107)
	at io.trino.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.trino.sql.analyzer.StatementAnalyzer$Visitor.process(StatementAnalyzer.java:464)
	at io.trino.sql.analyzer.StatementAnalyzer.analyze(StatementAnalyzer.java:427)
	at io.trino.sql.analyzer.Analyzer.analyze(Analyzer.java:79)
	at io.trino.sql.analyzer.Analyzer.analyze(Analyzer.java:71)
	at io.trino.execution.SqlQueryExecution.analyze(SqlQueryExecution.java:269)
	at io.trino.execution.SqlQueryExecution.<init>(SqlQueryExecution.java:193)
	at io.trino.execution.SqlQueryExecution$SqlQueryExecutionFactory.createQueryExecution(SqlQueryExecution.java:808)
	at io.trino.dispatcher.LocalDispatchQueryFactory.lambda$createDispatchQuery$0(LocalDispatchQueryFactory.java:139)
	at io.trino.$gen.Trino_392_mi_SNAPSHOT____20220811_090358_2.call(Unknown Source)
	at com.google.common.util.concurrent.TrustedListenableFutureTask$TrustedFutureInterruptibleTask.runInterruptibly(TrustedListenableFutureTask.java:131)
	at com.google.common.util.concurrent.InterruptibleTask.run(InterruptibleTask.java:74)
	at com.google.common.util.concurrent.TrustedListenableFutureTask.run(TrustedListenableFutureTask.java:82)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

[xiacongling]

After stepping into the code, I find that the schema is null after this line:

trino/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

Line 329 in bfb1c63

tableSchema = table.schemas().get(table.snapshot(snapshotId).schemaId());

Our team use Spark with Iceberg lib version of 0.12 and it may not support to write schema-id for snapshots in metadata json files.

And I notice that in Iceberg code comment, the schema id can be null for some reasons:

  /**
   * Return the id of the schema used when this snapshot was created, or null if this information is
   * not available.
   *
   * @return schema id associated with this snapshot
   */

Should we use current schema when schema-id is absent?

[xiacongling]

Current Iceberg community solution to null schema-id is to use the current schema. We can use the native implementation to avoid this NullPointerException.

https://github.com/apache/iceberg/blob/5a15efc070ab59eeda6343998aa065c0c9892c5c/core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java#L355-L369

  public static Schema schemaFor(Table table, long snapshotId) {
    Snapshot snapshot = table.snapshot(snapshotId);
    Preconditions.checkArgument(snapshot != null, "Cannot find snapshot with ID %s", snapshotId);
    Integer schemaId = snapshot.schemaId();

    // schemaId could be null, if snapshot was created before Iceberg added schema id to snapshot
    if (schemaId != null) {
      Schema schema = table.schemas().get(schemaId);
      Preconditions.checkState(schema != null, "Cannot find schema with schema id %s", schemaId);
      return schema;
    }

    // TODO: recover the schema by reading previous metadata files
    return table.schema();
  }