yujun777 commented on code in PR #69850:
URL: https://github.com/apache/doris/pull/69850#discussion_r4419779499
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTableWrapper.java:
##########
@@ -190,6 +190,33 @@ public MaterializedIndex getBaseIndex() {
return originTable.getBaseIndex();
}
+ /**
+ * Row counts are derived from the partitions of the table, and this
wrapper holds none of its
+ * own -- it only delegates partition lookups to the table it wraps. The
inherited methods
+ * iterate the wrapper's private partition map, so every scan over a
wrapper would be estimated
+ * at zero rows, which the statistics calculator raises to one; a reset
read of a
+ * four-billion-row table would then look like a one-row build side to the
optimizer.
+ *
+ * <p>Delegating answers the read with the table it is over: a reset read,
and a snapshot read
+ * whose offsets already reached the end, see exactly that table's
content, and a snapshot read
+ * of an older offset is bounded by it (the table only grows for the
duplicate-key tables this
+ * wrapper serves in that way).
+ */
+ @Override
+ public long getRowCountForIndex(long indexId, boolean strict) {
+ return originTable.getRowCountForIndex(indexId, strict);
+ }
+
+ @Override
+ public long getRowCountForPartitionIndex(long partitionId, long indexId,
boolean strict) {
+ return originTable.getRowCountForPartitionIndex(partitionId, indexId,
strict);
+ }
+
+ @Override
+ public int getPartitionNum() {
Review Comment:
Keeping the delegation and the changelog estimate as is.
`OlapChangelogTableWrapper` still answers `UNKNOWN_ROW_COUNT` for both
row-count methods, so the change here is only which of the statistics
calculator's two generic lower bounds applies to a partition-pruned changelog
scan: one row for the relation before, one row per selected partition after.
Both numbers describe a quantity that is unknown by design, both stay orders of
magnitude below `broadcast_row_count_limit`, and neither moves the join or
broadcast choice -- the estimate does not grow with the size of the change
range, which is what would matter. Suppressing the per-partition fallback for
this wrapper would put a catalog-type check into the generic statistics path,
so I'd rather track "unknown relations should not be broadcast build sides"
separately.
I did not add a multi-partition incremental EXPLAIN case: the value it would
pin down is exactly the fabricated estimate (1 vs number of selected
partitions), and a regression that asserts an estimate the change does not
intend to make more accurate would be a maintenance liability. If you'd rather
have the old one-row behaviour preserved bit-for-bit, say so and I'll bypass
the fallback for the changelog wrapper instead.
##########
regression-test/suites/changelog_p0/test_stream_scan_row_count.groovy:
##########
@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.awaitility.Awaitility
+
+import static java.util.concurrent.TimeUnit.SECONDS
+
+/**
+ * The optimizer must estimate a stream read from the table it reads, not at
one row.
+ *
+ * <p>A stream read is planned over a lightweight wrapper of the base table.
The wrapper holds no
+ * partitions of its own, so a row count derived from its own partitions
collapsed to zero and the
+ * statistics calculator raised that to one: a stream read of a large table
looked like a one-row
+ * scan, and the optimizer could pick the real large side as a broadcast build.
+ *
+ * <p>This suite runs in both shared-nothing and cloud mode, and that matters:
in cloud every read
+ * of a stream wraps the base table once the cloud read states are installed,
so even a reset read
+ * goes through the wrapper; in shared-nothing a reset read scans the real
table and only a
+ * snapshot read uses a wrapper. Both must be estimated from the table.
+ */
+suite("test_stream_scan_row_count", "nonConcurrent") {
+ sql "DROP STREAM IF EXISTS stats_stream FORCE"
+ sql "DROP TABLE IF EXISTS stats_stream_t"
+ sql """
+ CREATE TABLE stats_stream_t (
+ id BIGINT NOT NULL,
+ v BIGINT
+ )
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "changelog.enable" = "true",
+ "changelog.ttl_seconds" = "86400"
+ )
+ """
+ // Five rows exist when the stream is created; the last two are written
afterwards, so the
+ // snapshot image holds five rows while the table holds seven.
+ sql "INSERT INTO stats_stream_t VALUES (1,10),(2,20),(3,30),(4,40),(5,50)"
+ sql "CREATE STREAM stats_stream ON TABLE stats_stream_t PROPERTIES
('show_initial_rows'='false')"
Review Comment:
Fixed in 0276aff: `sql "sync"` now follows both inserts, so the rows are
published before `CREATE STREAM` records the stream's offset and before the
post-create inserts are explained.
--
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]
