github-actions[bot] commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4002635648
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -135,6 +138,11 @@ public void setEnableMappingTimestampTz(boolean enable) {
public void modifyCatalogProps(Map<String, String> props) {
synchronized (this) {
properties.putAll(props);
+ if (props.containsKey(ENABLE_MAPPING_VARBINARY)) {
+ // Normalize ALTER CATALOG updates so the compatibility marker
cannot revive the
+ // removed STRING mapping after a rolling downgrade.
+ properties.put(ENABLE_MAPPING_VARBINARY, "true");
Review Comment:
[P1] Persist the normalized varbinary marker in ALTER logs
`CatalogMgr.alterCatalogProps` stores the caller's `newProperties` map in
`CatalogLog` before replaying it, but this branch only overwrites the private
live `properties` map. Therefore `ALTER CATALOG ... SET PROPERTIES
("enable.mapping.varbinary"="false")` leaves the new FE at true while
journaling false; an older FE that replays the entry and takes over during
rollback can restore the removed STRING mapping. Please normalize the map
written to `CatalogLog` (or reject/overwrite this key before logging) and cover
the serialized replay path, not only the live `CatalogProperty` object.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -274,9 +274,9 @@ public void setDefaultPropsIfMissing(boolean isReplay) {
// set default value to true, no matter is replaying or not.
// After 4.0, all external catalogs will use meta cache by default.
catalogProperty.addProperty(USE_META_CACHE,
String.valueOf(DEFAULT_USE_META_CACHE));
- if
(catalogProperty.getOrDefault(CatalogProperty.ENABLE_MAPPING_VARBINARY,
"").isEmpty()) {
- catalogProperty.setEnableMappingVarbinary(false);
- }
+ // Persist the binary-safe value so an older FE can still plan
compatible scans after a
+ // rollback or during a rolling upgrade, even though new code no
longer exposes a switch.
+ catalogProperty.setEnableMappingVarbinary(true);
Review Comment:
[P1] Replicate the marker migration for existing catalogs
This setter only changes the FE process that constructs or deserializes the
catalog; loading a pre-upgrade catalog does not emit an edit-log record. An
already-running old FE therefore keeps the persisted `false` marker and can
still plan the same binary column as STRING during the rolling upgrade
(readable followers execute queries locally), and rolling that FE back before
it loads a new image restores the same behavior. Please migrate existing
catalogs through an idempotent replicated property update, and cover a
pre-upgrade catalog on a mixed-version follower.
##########
regression-test/suites/external_table_p0/hive/test_parquet_lazy_mat_profile.groovy:
##########
@@ -267,374 +274,27 @@ suite("test_parquet_lazy_mat_profile",
"p0,external,hive,external_docker,externa
- def test_true_true = {
- sql """ set enable_parquet_filter_by_min_max = true; """
- sql """ set enable_parquet_lazy_materialization = true; """
-
- def metrics = q1()
- logger.info("metrics = ${metrics}")
- assertEquals("99", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("1", metrics["RawRowsRead"])
- assertEquals("99", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("99", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q2()
- logger.info("metrics = ${metrics}")
- assertEquals("99", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("1", metrics["RawRowsRead"])
- assertEquals("99", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("99", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q3()
- logger.info("metrics = ${metrics}")
- assertEquals("100", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("0", metrics["RawRowsRead"])
- assertEquals("100", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("100", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("0", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q4()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("20", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.279K (7279)", metrics["FilteredRowsByPage"])
- assertEquals("21", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q5()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("28", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.258K (7258)", metrics["FilteredRowsByPage"])
- assertEquals("42", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q6()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("1", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q7()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("19", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.279K (7279)", metrics["FilteredRowsByPage"])
- assertEquals("21", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
- }
-
-
- def test_true_false = {
- sql """ set enable_parquet_filter_by_min_max = true; """
- sql """ set enable_parquet_lazy_materialization = false; """
- // in v2 lazy materialization is always enabled.
- sql """ set enable_file_scanner_v2=false; """
-
- def metrics = q1()
- logger.info("metrics = ${metrics}")
- assertEquals("99", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("1", metrics["RawRowsRead"])
- assertEquals("99", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("99", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q2()
- logger.info("metrics = ${metrics}")
- assertEquals("99", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("1", metrics["RawRowsRead"])
- assertEquals("99", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("99", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q3()
- logger.info("metrics = ${metrics}")
- assertEquals("100", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("0", metrics["RawRowsRead"])
- assertEquals("100", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("100", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("0", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q4()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.279K (7279)", metrics["FilteredRowsByPage"])
- assertEquals("21", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q5()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.258K (7258)", metrics["FilteredRowsByPage"])
- assertEquals("42", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q6()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q7()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("7.279K (7279)", metrics["FilteredRowsByPage"])
- assertEquals("21", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
- }
-
-
- def test_false_false = {
- sql """ set enable_parquet_filter_by_min_max = false; """
- sql """ set enable_parquet_lazy_materialization = false; """
-
- def metrics = q1()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q2()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q3()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q4()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("7.3K (7300)", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q5()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("7.3K (7300)", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q6()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("7.3K (7300)", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q7()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("0", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("7.3K (7300)", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
- }
-
-
- def test_false_true = {
- sql """ set enable_parquet_filter_by_min_max = false; """
- sql """ set enable_parquet_lazy_materialization = true; """
-
- def metrics = q1()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("99", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q2()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("99", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q3()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertEquals("100", metrics["FilteredRowsByLazyRead"])
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertEquals("100", metrics["RawRowsRead"])
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("100", metrics["RowGroupsReadNum"])
- assertEquals("100", metrics["RowGroupsTotalNum"])
-
- metrics = q4()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertTrue(metrics["FilteredRowsByLazyRead"].contains("7299"))
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q5()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertTrue(metrics["FilteredRowsByLazyRead"].contains("7286"))
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q6()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertTrue(metrics["FilteredRowsByLazyRead"].contains("1"))
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
-
- metrics = q7()
- logger.info("metrics = ${metrics}")
- assertEquals("0", metrics["FilteredRowsByGroup"])
- assertTrue(metrics["FilteredRowsByLazyRead"].contains("7298"))
- assertEquals("0", metrics["FilteredRowsByPage"])
- assertTrue(metrics["RawRowsRead"].contains("7300"))
- assertEquals("0", metrics["RowGroupsFiltered"])
- assertEquals("0", metrics["RowGroupsFilteredByBloomFilter"])
- assertEquals("0", metrics["RowGroupsFilteredByMinMax"])
- assertEquals("1", metrics["RowGroupsReadNum"])
- assertEquals("1", metrics["RowGroupsTotalNum"])
+ // Versioned Parquet plans always use V2, including when the session
toggle is false.
+ // V2 filters predicates before materializing output regardless of the
legacy lazy flag.
+ for (boolean scannerV2 : [false, true]) {
+ sql "set enable_file_scanner_v2=${scannerV2}"
+ for (boolean minMax : [false, true]) {
+ sql "set enable_parquet_filter_by_min_max=${minMax}"
+ for (boolean lazy : [false, true]) {
+ sql "set enable_parquet_lazy_materialization=${lazy}"
+ for (def query : [q1, q2, q3, q4, q5, q6, q7]) {
+ def metrics = query()
+ long raw = metricValueAsLong(metrics["RawRowsRead"])
+ long selected =
metricValueAsLong(metrics["ReaderSelectRows"])
+ long filtered =
metricValueAsLong(metrics["RowsFilteredByConjunct"])
+ long lazyFiltered =
metricValueAsLong(metrics["FilteredRowsByLazyRead"])
+ assertTrue(raw >= 0 && selected >= 0 && filtered >= 0)
Review Comment:
[P2] Keep assertions that prove Parquet pruning occurs
These replacement checks only verify profile accounting. If V2 stops pruning
all 100 row groups, or stops page-index pruning and reads all 7,300 rows,
conjunct filtering still makes `raw == selected + filtered` and the lazy-filter
bound pass. The deleted checks were the only assertions on
`RowGroupsFilteredByMinMax` and `FilteredRowsByPage`, while `q8` covers only
lazy filtering. Please retain relative V2 assertions that min/max and page
pruning actually reduce the scanned groups or rows.
--
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]
