github-actions[bot] commented on code in PR #68846:
URL: https://github.com/apache/doris/pull/68846#discussion_r4236059324
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -273,21 +284,20 @@ public IcebergConnector(Map<String, String> properties,
ConnectorContext context
// tableCache/commentCache discipline: session=user => no LIVE
cross-query metadata cache.
this.latestSnapshotCache = isUserSessionEnabled()
? null
- : new IcebergLatestSnapshotCache(
- metaCache,
resolveTableCacheTtlSecond(this.properties), DEFAULT_TABLE_CACHE_CAPACITY);
+ : new IcebergLatestSnapshotCache(metaCache,
tableFactsCacheSpec);
// PERF-01 cross-query RAW-table cache. Disabled (null) when the
catalog's credentials are
Review Comment:
[P2] Propagate the no-cache schema policy through the HMS gateway. For an
Iceberg table reached through an HMS catalog, the gateway copies these table
properties into its Iceberg sibling and forwards `beginQuerySnapshot`, so this
change makes the sibling's snapshot pin live. FE asks the outer `HiveConnector`
for `schemaCacheTtlSecondOverride`, however; Hive returns empty and can keep
serving the old name-keyed schema until cache refresh or invalidation. After an
external schema-only ALTER, the next query can pin the new schema ID while
binding old columns (and reject a newly added column). Please carry the
sibling's per-table no-cache schema decision through this gateway path without
disabling schema caching for ordinary Hive tables, and cover it in a gateway
test.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -733,10 +741,15 @@ public void invalidateAll() {
* {@code schema.cache.ttl-second}), so a no-cache catalog ({@code
ttl-second=0}) serves FRESH schema after
* external DDL (mirrors {@code
PaimonConnector.schemaCacheTtlSecondOverride}). Absent -> no override (engine
* default TTL). Do NOT reuse {@link #resolveTableCacheTtlSecond}, which
substitutes the 24h default for a
- * blank value and would defeat the engine default.
+ * blank value and would defeat the engine default. A table entry turned
off by {@code enable=false} or
+ * {@code capacity=0} is a no-cache catalog too: its latest snapshot is
read live, so the schema it names must
+ * not come from the name-keyed schema cache either.
*/
@Override
public OptionalLong schemaCacheTtlSecondOverride() {
+ if (!tableFactsCacheSpec.isEnable() ||
tableFactsCacheSpec.getCapacity() == 0) {
+ return OptionalLong.of(0L);
+ }
Review Comment:
[P2] Bind the live schema from the query's pinned table. With either new
table-cache disable setting, this zero override makes
`PluginDrivenMvccExternalTable.materializeLatest` read schema through
`initSchema()` in a separate cross-statement scope after `beginQuerySnapshot`
has frozen table generation G0 in the SQL statement scope. Iceberg leaves
`retainSchema=false`; if external `ALTER TABLE ADD COLUMN` lands between those
loads, FE can bind G1 columns while scan `getColumnHandles` still reads G0 and
silently omits the new column handle. Please retain the schema with the latest
pin or bind from the pinned statement table, and test an ALTER interleaving.
The older ttl-second=0 path has the same race; these new settings now enter it
too.
--
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]
