Copilot commented on code in PR #57771:
URL: https://github.com/apache/doris/pull/57771#discussion_r2508669364
##########
be/src/vec/exec/format/parquet/parquet_predicate.h:
##########
@@ -161,9 +162,31 @@ class ParquetPredicate {
std::function<bool(ParquetPredicate::ColumnStat*, const int)>*
get_stat_func;
};
- static Status get_min_max_value(const FieldSchema* col_schema, const
std::string& encoded_min,
- const std::string& encoded_max, const
cctz::time_zone& ctz,
- Field* min_field, Field* max_field) {
+ struct PageIndexStat {
+ // Indicates whether the page index information in this column can be
used.
+ bool available = false;
+ int64_t num_of_pages;
+ std::vector<std::string> encoded_min_value;
+ std::vector<std::string> encoded_max_value;
+ std::vector<bool> has_null;
+ std::vector<bool> is_all_null;
+ const FieldSchema* col_schema;
+
+ // Record the row range corresponding to each page.
+ std::vector<segment_v2::RowRange> ranges;
+ };
+
+ struct CachedPageIndexStat {
+ const cctz::time_zone* ctz;
+ std::map<int, PageIndexStat> stats;
+ std::function<bool(PageIndexStat**, int)> get_stat_func;
+ };
+
+ // The encoded Parquet min-max value is parsed into `fields`;
Review Comment:
The comment "The encoded Parquet min-max value is parsed into `fields`" has
an extra space between "into" and the backtick. It should be a single space.
```suggestion
// The encoded Parquet min-max value is parsed into `fields`;
```
##########
regression-test/suites/external_table_p0/hive/test_hive_page_index.groovy:
##########
@@ -0,0 +1,98 @@
+// 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.
+
+suite("test_hive_page_index",
"p0,external,hive,external_docker,external_docker_hive") {
+
+
+ String enabled = context.config.otherConfigs.get("enableHiveTest")
+ if (enabled == null || !enabled.equalsIgnoreCase("true")) {
+ logger.info("diable Hive test.")
Review Comment:
The spelling "diable" should be "disable".
##########
regression-test/suites/external_table_p0/hive/test_hive_page_index.groovy:
##########
@@ -0,0 +1,98 @@
+// 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.
+
+suite("test_hive_page_index",
"p0,external,hive,external_docker,external_docker_hive") {
+
+
+ String enabled = context.config.otherConfigs.get("enableHiveTest")
+ if (enabled == null || !enabled.equalsIgnoreCase("true")) {
+ logger.info("diable Hive test.")
+ return;
+ }
+
+ for (String hivePrefix : ["hive3"]) {
+ String hms_port = context.config.otherConfigs.get(hivePrefix +
"HmsPort")
+ String hdfs_port = context.config.otherConfigs.get(hivePrefix +
"HdfsPort")
+ String externalEnvIp = context.config.otherConfigs.get("externalEnvIp")
+
+ String catalog_name = "${hivePrefix}_test_hive_page_index"
+
+ sql """drop catalog if exists ${catalog_name}"""
+ sql """create catalog if not exists ${catalog_name} properties (
+ "type"="hms",
+ 'hive.metastore.uris' = 'thrift://${externalEnvIp}:${hms_port}'
+ );"""
+
+ sql """switch ${catalog_name}"""
+ sql """ use `default` """
+
+ order_qt_q1 """select * from alltypes_tiny_pages_plain where id < 30
or id > 7270 and bigint_col = 20;"""
+ order_qt_q2 """select * from alltypes_tiny_pages_plain where id > 2300
or id < 2310 and bigint_col = 20 ;"""
+ order_qt_q3 """select * from alltypes_tiny_pages_plain where id < 100
or month = 7 or year = 2010;"""
+ order_qt_q4 """select * from alltypes_tiny_pages_plain where
bigint_col = 0 or month = 7;"""
+ order_qt_q5 """select * from alltypes_tiny_pages_plain where
bigint_col = 0 or year = 2010;"""
+ order_qt_q6 """select * from alltypes_tiny_pages_plain where
bigint_col = 0 or month = 7 or year = 2011;"""
+ order_qt_q7 """select * from alltypes_tiny_pages_plain where
date_string_col = '02/02/09' or month = 1;"""
+ order_qt_q8 """select * from alltypes_tiny_pages_plain where
date_string_col = '02/02/09' or year = 2010;"""
+ order_qt_q9 """select * from alltypes_tiny_pages_plain where
timestamp_col < '2009-12-08 00:19:03' or smallint_col = 9;"""
+ order_qt_q10 """select * from alltypes_tiny_pages_plain where
(double_col > 70 and double_col < 71) or ( month = 8 and year = 2010 and
bigint_col = 0);"""
+
+
+
+ order_qt_q13 """select * from decimals_1_10 where d_1 = 1 or d_10 =
2;"""
+ order_qt_q14 """select * from decimals_1_10 where d_1 = 3 or d_10 =
4;"""
+ order_qt_q15 """select * from decimals_1_10 where d_1 = 5 or d_10 =
6;"""
+ order_qt_q16 """select * from decimals_1_10 where d_1 = 9 or d_10 =
10;"""
+
+ order_qt_q17 """select * from decimals_1_10 where d_1 < 3 or d_10 >
7;"""
+ order_qt_q18 """select * from decimals_1_10 where d_1 > 2 or d_10 <
5;"""
+ order_qt_q19 """select * from decimals_1_10 where d_1 > 5 or d_10 <
3;"""
+
+ order_qt_q20 """select * from decimals_1_10 where d_1 = 2 or d_10 = 6
or d_1 = 8;"""
+ order_qt_q21 """select * from decimals_1_10 where (d_1 > 5 and d_10 <
9) or d_1 = 1;"""
+ order_qt_q22 """select * from decimals_1_10 where (d_1 > 2 and d_10 <
5) or (d_1 > 7);"""
+
+
+
+
+ order_qt_q23 """select * from alltypes_tiny_pages where id < 30 or id
> 7270 and bigint_col = 20;"""
+ order_qt_q24 """select * from alltypes_tiny_pages where id > 2300 or
id < 2310;"""
+ order_qt_q25 """select * from alltypes_tiny_pages where id < 100 or
month = 7 or year = 2010;"""
+ order_qt_q26 """select * from alltypes_tiny_pages where bigint_col = 0
or month = 7;"""
+ order_qt_q27 """select * from alltypes_tiny_pages where bigint_col = 0
or year = 2010;"""
+ order_qt_q28 """select * from alltypes_tiny_pages where bigint_col = 0
or month = 7 or year = 2011;"""
+ order_qt_q29 """select * from alltypes_tiny_pages where
date_string_col = '02/02/09' or month = 1;"""
+ order_qt_q30 """select * from alltypes_tiny_pages where timestamp_col
< '2009-12-08 00:19:03' or smallint_col = 9;"""
+ order_qt_q31 """select * from alltypes_tiny_pages where
date_string_col = '02/02/09' or year = 2010;"""
+ order_qt_q32 """select * from alltypes_tiny_pages where (double_col >
70 and double_col < 71) or ( month = 8 and year = 2010 and bigint_col = 0);"""
+
+
+ order_qt_q33 """ select * from decimals_1_10 where d_1 is null or d_10
is null ; """
+ order_qt_q33 """ select * from decimals_1_10 where d_1 is null"""
+ order_qt_q33 """ select * from decimals_1_10 where d_10 is null ; """
Review Comment:
Duplicate query identifiers detected. Lines 84, 85, and 86 all use the same
identifier `order_qt_q33`, but they are executing different queries. Each query
should have a unique identifier, such as `order_qt_q34` and `order_qt_q35` for
the second and third queries respectively.
##########
be/src/vec/exec/format/parquet/vparquet_reader.h:
##########
@@ -206,25 +204,42 @@ class ParquetReader : public GenericReader, public
ExprPushDownHelper {
RowGroupReader::PositionDeleteContext _get_position_delete_ctx(
const tparquet::RowGroup& row_group,
const RowGroupReader::RowGroupIndex& row_group_index);
- Status _init_row_groups(const bool& is_filter_groups);
void _init_system_properties();
void _init_file_description();
- // Page Index Filter
- bool _has_page_index(const std::vector<tparquet::ColumnChunk>& columns,
PageIndex& page_index);
+
// At the beginning of reading next row group, index should be loaded and
used to filter data efficiently.
- Status _process_page_index(const tparquet::RowGroup& row_group,
- const RowGroupReader::RowGroupIndex&
row_group_index,
- std::vector<RowRange>& candidate_row_ranges);
+ Status _process_page_index_filter(
+ const tparquet::RowGroup& row_group,
+ const RowGroupReader::RowGroupIndex& row_group_index,
+ const std::vector<std::unique_ptr<MutilColumnBlockPredicate>>&
push_down_pred,
+ RowRanges* candidate_row_ranges);
- // Row Group Filter
+ // check this range contain this tow group.
Review Comment:
The comment "check this range contain this tow group" has a spelling error.
"tow" should be "row".
```suggestion
// check this range contain this row group.
```
--
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]
