gfphoenix78 commented on code in PR #1599:
URL: https://github.com/apache/cloudberry/pull/1599#discussion_r2888348351
##########
src/backend/commands/analyzeutils.c:
##########
@@ -759,17 +759,32 @@ getNdvBySegHeapTuple(AttStatsSlot * *ndvbsSlots,
HeapTuple *heaptupleStats, floa
(void) get_attstatsslot(ndvbsSlots[i], heaptupleStats[i],
STATISTIC_KIND_NDV_BY_SEGMENTS, InvalidOid,
ATTSTATSSLOT_VALUES);
- if ((InvalidOid != ndvbsSlots[i]->valuetype && // result is not
empty
- // not empty partition with invalid ndvbs
- (relTuples[i] > 0 &&
DatumGetFloat8(ndvbsSlots[i]->values[0]) == 0)) ||
- // not empty partition without ndvbs
- (InvalidOid == ndvbsSlots[i]->valuetype && relTuples[i]
> 0)) {
- valid = false;
- break;
+ if (ndvbsSlots[i]->valuetype != FLOAT8OID)
+ {
+ /*
+ * NDV_BY_SEGMENTS slot not found or has unexpected
type.
+ * Non-empty partitions must have valid NDV_BY_SEGMENTS;
+ * empty partitions (relTuples == 0) can be skipped.
+ */
+ if (relTuples[i] > 0)
+ {
+ valid = false;
+ break;
+ }
+ free_attstatsslot(ndvbsSlots[i]);
+ pfree(ndvbsSlots[i]);
+ ndvbsSlots[i] = NULL;
+ continue;
}
- Assert(ndvbsSlots[i]->valuetype == FLOAT8OID);
Assert(ndvbsSlots[i]->nvalues == 1);
+
+ /* Non-empty partition with zero NDV is suspicious */
+ if (relTuples[i] > 0 &&
DatumGetFloat8(ndvbsSlots[i]->values[0]) == 0)
+ {
+ valid = false;
+ break;
+ }
}
return valid;
Review Comment:
All of the ndvbsSlots could be release if the valid becomes false. On the
other side, the ndvbsSlots and its pointer array could be also freed in
aggregate_leaf_partition_ndvbs no matter it's valid or not.
##########
src/backend/commands/analyzeutils.c:
##########
@@ -759,17 +759,32 @@ getNdvBySegHeapTuple(AttStatsSlot * *ndvbsSlots,
HeapTuple *heaptupleStats, floa
(void) get_attstatsslot(ndvbsSlots[i], heaptupleStats[i],
STATISTIC_KIND_NDV_BY_SEGMENTS, InvalidOid,
ATTSTATSSLOT_VALUES);
- if ((InvalidOid != ndvbsSlots[i]->valuetype && // result is not
empty
- // not empty partition with invalid ndvbs
- (relTuples[i] > 0 &&
DatumGetFloat8(ndvbsSlots[i]->values[0]) == 0)) ||
- // not empty partition without ndvbs
- (InvalidOid == ndvbsSlots[i]->valuetype && relTuples[i]
> 0)) {
- valid = false;
- break;
+ if (ndvbsSlots[i]->valuetype != FLOAT8OID)
+ {
+ /*
+ * NDV_BY_SEGMENTS slot not found or has unexpected
type.
+ * Non-empty partitions must have valid NDV_BY_SEGMENTS;
+ * empty partitions (relTuples == 0) can be skipped.
+ */
+ if (relTuples[i] > 0)
+ {
+ valid = false;
+ break;
+ }
+ free_attstatsslot(ndvbsSlots[i]);
+ pfree(ndvbsSlots[i]);
+ ndvbsSlots[i] = NULL;
+ continue;
}
- Assert(ndvbsSlots[i]->valuetype == FLOAT8OID);
Review Comment:
This assert may keep here.
--
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]
