github-actions[bot] commented on code in PR #68860:
URL: https://github.com/apache/doris/pull/68860#discussion_r4236402847


##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -308,11 +308,9 @@ public List<String> listTables(String databaseName) throws 
Catalog.DatabaseNotEx
          */
         @Override
         public Table getTable(Identifier identifier) throws 
Catalog.TableNotExistException {
-            // Doris owns the outer table-metadata cache and only reaches this 
method on an outer-cache
-            // load. Paimon's CachingCatalog owns a second cache for the Table 
and its lower-level readers.
-            // Invalidate that inner entry before reloading, otherwise REFRESH 
or outer-cache expiry can
-            // immediately repopulate Doris with the same stale bucket, path, 
or table options.
-            catalog.invalidateTable(identifier);
+            // Every metadata, scan and write lookup lands here, several times 
per statement. The wrapped

Review Comment:
   [P1] Keep the snapshot pin in step with the table cache. With 
`paimon.cache-enabled=false`, the table is reloaded on every lookup but 
`PaimonLatestSnapshotCache` still holds the prior snapshot id for its default 
24-hour access TTL. Likewise, when a configured 
`paimon.cache.expire-after-write` reloads the table, the old snapshot id 
survives; `beginQuerySnapshot` keeps pinning scans to it, and frequent reads 
can extend that staleness indefinitely after an external INSERT. The removed 
invalidation previously cleared both entries. Disable or expire the snapshot 
cache whenever the table cache is disabled or expires, and cover both settings 
with external-write reads.



##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -308,11 +308,9 @@ public List<String> listTables(String databaseName) throws 
Catalog.DatabaseNotEx
          */
         @Override
         public Table getTable(Identifier identifier) throws 
Catalog.TableNotExistException {
-            // Doris owns the outer table-metadata cache and only reaches this 
method on an outer-cache
-            // load. Paimon's CachingCatalog owns a second cache for the Table 
and its lower-level readers.
-            // Invalidate that inner entry before reloading, otherwise REFRESH 
or outer-cache expiry can
-            // immediately repopulate Doris with the same stale bucket, path, 
or table options.
-            catalog.invalidateTable(identifier);
+            // Every metadata, scan and write lookup lands here, several times 
per statement. The wrapped
+            // catalog is the Doris table cache (PaimonMetaCacheCatalog; the 
SDK CachingCatalog is disabled),
+            // so this must not invalidate: REFRESH, Doris writes and DDL 
invalidate through CatalogMetaCache.

Review Comment:
   [P2] Resolve the current schema for INSERT targets. After an external `ALTER 
TABLE ... ADD COLUMN`, this retained Table can still have the old 
`schema().fields()`. `PaimonWritePlanProvider.getWriteColumns` uses those 
fields for the statement's writer schema, so an INSERT naming the new column 
fails as `Unknown column` until table expiry or REFRESH, even when 
`schema.cache.ttl-second=0` lets DESC show the new column. The previous 
per-lookup invalidation reloaded the target Table. Read the live target schema 
at the writer boundary, and cover external ALTER followed by Doris INSERT.



-- 
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]

Reply via email to