deniskuzZ commented on code in PR #6789:
URL: https://github.com/apache/hive/pull/6789#discussion_r4247827362
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveTableUtil.java:
##########
@@ -259,6 +261,71 @@ public static Table deserializeTable(Configuration config,
String name) {
return table;
}
+ /**
+ * Resolves the Iceberg {@link Table} for split generation ({@code
IcebergInputFormat#getSplits}).
+ *
+ * <p>Serialized tables ({@link SerializableTable}) only carry a metadata
snapshot and produce
+ * {@link org.apache.iceberg.DataTableScan} (client-side manifest planning).
When REST catalog
+ * server-side scan planning is enabled, reload the live table from the
catalog so
+ * {@code table.newScan()} returns {@link
org.apache.iceberg.rest.RESTTableScan} and issues
+ * {@code POST /plan} on the REST server.
+ *
+ * <p>Intra-transaction read-after-write ({@link
InputFormatConfig#TABLE_METADATA_LOCATION}) still
+ * uses the deserialized snapshot so uncommitted metadata is visible.
+ */
+ public static Table resolveTableForScanPlanning(Configuration conf, String
tableIdentifier) {
+ if (shouldReloadForServerSideScanPlanning(conf)) {
+ Table table = Catalogs.loadTable(conf);
+ checkAndSetIoConfig(conf, table);
+ IcebergVendedCredentialUtil.applyFromJobConf(table,
resolveCatalogName(conf), conf);
+ return table;
+ }
+
+ Table table = deserializeTable(conf, tableIdentifier);
+ if (table == null) {
+ table = Catalogs.loadTable(conf);
+ checkAndSetIoConfig(conf, table);
+ }
+ return table;
+ }
+
+ private static boolean shouldReloadForServerSideScanPlanning(Configuration
conf) {
Review Comment:
Design question on where the server-vs-client planning decision lives.
Right now `shouldReloadForServerSideScanPlanning` re-derives it in the Tez
AM from catalog name/default catalog, catalog type, the Hive flag and the
catalog `scan-planning-mode`, and most of those only exist in the AM because
`RestCatalogScanPlanningUtil` copies them from the HS2 session through two
hooks (`overlayTableProperties` and `configureJobConf`). If any of that
propagation misses, we silently fall back to client-side planning; the
IntegrationTest `executorJobConfWithoutPropagationUsesSerializedTable` actually
encodes that behavior.
HS2 has all of this at compile time (session config, the loaded table, its
current snapshot). Would it be simpler to decide once there and put an explicit
marker in the job conf, e.g. `iceberg.mr.server-side-scan-planning=true` plus
the snapshot id to pin? Then `getSplits` only checks that flag, and:
- the AM no longer depends on re-deriving catalog type/mode from copied
properties,
- propagation is reduced to the connection settings needed for `loadTable`
(and can exclude secrets),
- if the flag is set but the reload fails, we can fail or at least WARN
instead of silently degrading,
- the pinned snapshot keeps the scan consistent with what the query was
compiled against.
--
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]
