github-actions[bot] commented on code in PR #66307:
URL: https://github.com/apache/doris/pull/66307#discussion_r3747349261
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -2637,6 +2650,16 @@ private Status atomicReplaceOlapTables(Database db,
boolean isReplay) {
newOlapTbl.checkAndSetName(originName, false);
db.unregisterTable(originName);
db.registerTable(newOlapTbl);
+
Env.getCurrentEnv().getConstraintManager().dropTableConstraints(
+ new TableNameInfo(
+ InternalCatalog.INTERNAL_CATALOG_NAME,
db.getFullName(), aliasName));
+ TableNameInfo originTableInfo = new TableNameInfo(
+ InternalCatalog.INTERNAL_CATALOG_NAME,
db.getFullName(), originName);
+
Env.getCurrentEnv().getConstraintManager().dropTableConstraints(originTableInfo);
Review Comment:
[P1] Do not cascade FKs owned by other tables
Atomic restore can run with `force_replace = false`, but this calls the
all-types `dropTableConstraints`. If the origin has a PK referenced from table
B, `cleanupConstraintReferences` silently removes B's FK; the following
`restoreTableConstraints` rebuilds mappings only, so that cross-table metadata
cannot return. Ordinary non-force DROP and non-swap REPLACE reject a referenced
origin PK instead of cascading it. Please give atomic restore the same
reference safeguard (or otherwise transition constraints without deleting
entries owned by unrelated live tables), and add a referenced-origin
atomic-restore test.
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -123,31 +142,79 @@ public void invalidateAboutTable(TableIf tableIf) {
);
}
}
+ return invalidateTableName;
+ }
- for (Entry<String, SqlCacheContext> kv : sqlCaches.asMap().entrySet())
{
- String key = kv.getKey();
- SqlCacheContext context = kv.getValue();
- for (Entry<FullTableName, TableVersion> nameToVersion :
context.getUsedTables().entrySet()) {
- FullTableName tableName = nameToVersion.getKey();
- TableVersion tableVersion = nameToVersion.getValue();
- if (tableVersion.id == tableIf.getId()) {
- invalidateKeys.add(key);
- break;
+ /** Invalidate table caches and fence publication using a persisted
qualified name. */
+ public void invalidateAboutTableAndFencePublication(TableNameInfo
tableNameInfo) {
+ invalidateAboutTable(-1L, new FullTableName(
+ tableNameInfo.getCtl(), tableNameInfo.getDb(),
tableNameInfo.getTbl()), true);
+ }
+
+ private void invalidateAboutTable(long tableId, FullTableName
invalidateTableName, boolean fencePublication) {
+ publicationLock.writeLock().lock();
+ try {
+ if (fencePublication) {
+ long invalidationSequence =
publicationSequence.incrementAndGet();
+ if (tableId >= 0) {
+ tableIdInvalidationSequences.put(tableId,
invalidationSequence);
Review Comment:
[P2] Reclaim publication fences for retired tables
Every fenced mapping ADD/DROP records both the table ID and qualified name
in these plain maps, but cache eviction, cache reconfiguration, table/database
drop, and rename never remove them. The only clear is `invalidateAll`, whose
sole production caller is cloud-only, so in shared-nothing mode a CREATE -> ADD
mapping -> DROP FORCE loop with fresh names/IDs grows FE heap forever
regardless of the configured SQL-cache size. Please add a race-safe
retirement/bounding scheme for fence entries and a churn test that still
rejects publishers whose baseline predates invalidation.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java:
##########
@@ -331,7 +336,9 @@ public Void visitPhysicalUnion(PhysicalUnion union,
PlanContext context) {
if (union.getConstantExprsList().isEmpty()) {
// translate requestPropertyFromParent to other children's request.
DistributionSpec distributionRequestFromParent =
requestPropertyFromParent.getDistributionSpec();
- if (distributionRequestFromParent instanceof DistributionSpecHash)
{
+ if (distributionRequestFromParent instanceof DistributionSpecHash
+ && ((DistributionSpecHash)
distributionRequestFromParent).getShuffleType()
+ != ShuffleType.COLOCATE_MAPPING_REQUIRE) {
Review Comment:
[P1] Block mapping requests for INTERSECT and EXCEPT too
This guard applies only to `PhysicalUnion`; `PhysicalIntersect` and
`PhysicalExcept` dispatch to the generic set-operation visitor, which still
forwards `COLOCATE_MAPPING_REQUIRE` to every child. A mapping-preserving
Aggregate can satisfy that request with `DistributionSpecStorageAny` plus a
hidden `NaturalDistributionMappingSpec`. The set-op regulator then takes its
hash-required branch and unconditionally casts that actual child distribution
to `DistributionSpecHash`, raising `ClassCastException` instead of falling back
to the ordinary set-operation shuffle. Please reject this non-enforceable
request in the generic set-operation path as well, with enabled-feature
INTERSECT and EXCEPT coverage.
--
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]
