You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by li...@apache.org on 2020/04/10 02:22:28 UTC
[incubator-doris] branch master updated: Fix bug when use
ZoneMap/BloomFiter on column with REPLACE/REPLACE_IF_NOT_NULL (#3288)
This is an automated email from the ASF dual-hosted git repository.
lichaoyong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new 3086790 Fix bug when use ZoneMap/BloomFiter on column with REPLACE/REPLACE_IF_NOT_NULL (#3288)
3086790 is described below
commit 3086790e06d961fc5dee00cfee762bfca6276b3b
Author: lichaoyong <li...@baidu.com>
AuthorDate: Fri Apr 10 10:22:21 2020 +0800
Fix bug when use ZoneMap/BloomFiter on column with REPLACE/REPLACE_IF_NOT_NULL (#3288)
Now, column with REPLACE/REPLACE_IF_NOT_NULL can be filtered by ZoneMap/BloomFilter
when the rowset is base(version starts with zero). Always we think is an optimization.
But when some case, it will occurs bug.
create table test(
k1 int,
v1 int replace,
v2 int sum
);
If I have two records on different two versions
1 2 2 on version [0-10]
1 3 1 on version 11
If I perform a query
select * from test where k1 = 1 and v1 = 3;
The result will be 1 3 1, this is not right because of the first record is filtered.
The right answer is 1 3 3, the v2 should be summed.
Remove this optimization is necessity to make the result is right.
---
be/src/olap/rowset/beta_rowset_writer.cpp | 1 -
be/src/olap/rowset/segment_reader.cpp | 10 ++--------
be/src/olap/rowset/segment_v2/segment_writer.cpp | 16 ----------------
be/src/olap/rowset/segment_v2/segment_writer.h | 2 --
be/test/olap/rowset/segment_v2/segment_test.cpp | 4 +---
.../org/apache/doris/common/util/PropertyAnalyzer.java | 4 +---
.../org/apache/doris/common/PropertyAnalyzerTest.java | 4 ++--
7 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/be/src/olap/rowset/beta_rowset_writer.cpp b/be/src/olap/rowset/beta_rowset_writer.cpp
index 11e2d78..c75f056 100644
--- a/be/src/olap/rowset/beta_rowset_writer.cpp
+++ b/be/src/olap/rowset/beta_rowset_writer.cpp
@@ -192,7 +192,6 @@ OLAPStatus BetaRowsetWriter::_create_segment_writer() {
DCHECK(wblock != nullptr);
segment_v2::SegmentWriterOptions writer_options;
- writer_options.whether_to_filter_value = _context.version.first == 0;
_segment_writer.reset(new segment_v2::SegmentWriter(
wblock.get(), _num_segment, _context.tablet_schema, writer_options));
_wblocks.push_back(std::move(wblock));
diff --git a/be/src/olap/rowset/segment_reader.cpp b/be/src/olap/rowset/segment_reader.cpp
index 8c74fa5..b6dddbf 100644
--- a/be/src/olap/rowset/segment_reader.cpp
+++ b/be/src/olap/rowset/segment_reader.cpp
@@ -491,10 +491,7 @@ OLAPStatus SegmentReader::_pick_row_groups(uint32_t first_block, uint32_t last_b
for (auto& i : _conditions->columns()) {
FieldAggregationMethod aggregation = _get_aggregation_by_index(i.first);
- bool is_continue = (aggregation == OLAP_FIELD_AGGREGATION_NONE
- || ((aggregation == OLAP_FIELD_AGGREGATION_REPLACE ||
- aggregation == OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL)
- && _segment_group->version().first == 0));
+ bool is_continue = (aggregation == OLAP_FIELD_AGGREGATION_NONE);
if (!is_continue) {
continue;
}
@@ -533,10 +530,7 @@ OLAPStatus SegmentReader::_pick_row_groups(uint32_t first_block, uint32_t last_b
for (uint32_t i : _load_bf_columns) {
FieldAggregationMethod aggregation = _get_aggregation_by_index(i);
- bool is_continue = (aggregation == OLAP_FIELD_AGGREGATION_NONE
- || ((aggregation == OLAP_FIELD_AGGREGATION_REPLACE ||
- aggregation == OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL)
- && _segment_group->version().first == 0));
+ bool is_continue = (aggregation == OLAP_FIELD_AGGREGATION_NONE);
if (!is_continue) {
continue;
}
diff --git a/be/src/olap/rowset/segment_v2/segment_writer.cpp b/be/src/olap/rowset/segment_v2/segment_writer.cpp
index 5a354f2..a93eaf9 100644
--- a/be/src/olap/rowset/segment_v2/segment_writer.cpp
+++ b/be/src/olap/rowset/segment_v2/segment_writer.cpp
@@ -71,25 +71,9 @@ Status SegmentWriter::init(uint32_t write_mbytes_per_sec) {
}
if (column.is_bf_column()) {
opts.need_bloom_filter = true;
- if ((column.aggregation() == OLAP_FIELD_AGGREGATION_REPLACE
- || column.aggregation() == OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL)
- && !_opts.whether_to_filter_value) {
- // if the column's Aggregation type is OLAP_FIELD_AGGREGATION_REPLACE or
- // OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL and the segment is not in base rowset,
- // do not write the bloom filter index because it is useless
- opts.need_bloom_filter = false;
- }
}
if (column.has_bitmap_index()) {
opts.need_bitmap_index = true;
- if ((column.aggregation() == OLAP_FIELD_AGGREGATION_REPLACE
- || column.aggregation() == OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL)
- && !_opts.whether_to_filter_value) {
- // if the column's Aggregation type is OLAP_FIELD_AGGREGATION_REPLACE or
- // OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL and the segment is not in base rowset,
- // do not write the bitmap index because it is useless
- opts.need_bitmap_index = false;
- }
}
std::unique_ptr<ColumnWriter> writer(
diff --git a/be/src/olap/rowset/segment_v2/segment_writer.h b/be/src/olap/rowset/segment_v2/segment_writer.h
index 2040714..4703f68 100644
--- a/be/src/olap/rowset/segment_v2/segment_writer.h
+++ b/be/src/olap/rowset/segment_v2/segment_writer.h
@@ -46,8 +46,6 @@ extern const uint32_t k_segment_magic_length;
struct SegmentWriterOptions {
uint32_t num_rows_per_block = 1024;
- // whether to filter value column against bloom filter/zone map
- bool whether_to_filter_value = false;
};
class SegmentWriter {
diff --git a/be/test/olap/rowset/segment_v2/segment_test.cpp b/be/test/olap/rowset/segment_v2/segment_test.cpp
index 5d40baf..a6a1638 100644
--- a/be/test/olap/rowset/segment_v2/segment_test.cpp
+++ b/be/test/olap/rowset/segment_v2/segment_test.cpp
@@ -1124,14 +1124,12 @@ TEST_F(SegmentReaderWriterTest, TestBloomFilterIndexUniqueModel) {
// for not base segment
SegmentWriterOptions opts1;
- opts1.whether_to_filter_value = false;
shared_ptr<Segment> seg1;
build_segment(opts1, schema, schema, 100, DefaultIntGenerator, &seg1);
- ASSERT_FALSE(column_contains_index(seg1->footer().columns(3), BLOOM_FILTER_INDEX));
+ ASSERT_TRUE(column_contains_index(seg1->footer().columns(3), BLOOM_FILTER_INDEX));
// for base segment
SegmentWriterOptions opts2;
- opts2.whether_to_filter_value = true;
shared_ptr<Segment> seg2;
build_segment(opts2, schema, schema, 100, DefaultIntGenerator, &seg2);
ASSERT_TRUE(column_contains_index(seg2->footer().columns(3), BLOOM_FILTER_INDEX));
diff --git a/fe/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
index e645727..f2a2a0d 100644
--- a/fe/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
+++ b/fe/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
@@ -284,9 +284,7 @@ public class PropertyAnalyzer {
throw new AnalysisException(type + " is not supported in bloom filter index. "
+ "invalid column: " + bfColumn);
} else if (column.isKey()
- || column.getAggregationType() == AggregateType.NONE
- || column.getAggregationType() == AggregateType.REPLACE
- || column.getAggregationType() == AggregateType.REPLACE_IF_NOT_NULL) {
+ || column.getAggregationType() == AggregateType.NONE) {
if (!bfColumnSet.add(bfColumn)) {
throw new AnalysisException("Reduplicated bloom filter column: " + bfColumn);
}
diff --git a/fe/src/test/java/org/apache/doris/common/PropertyAnalyzerTest.java b/fe/src/test/java/org/apache/doris/common/PropertyAnalyzerTest.java
index 45430e4..3aaaf20 100644
--- a/fe/src/test/java/org/apache/doris/common/PropertyAnalyzerTest.java
+++ b/fe/src/test/java/org/apache/doris/common/PropertyAnalyzerTest.java
@@ -49,10 +49,10 @@ public class PropertyAnalyzerTest {
columns.get(1).setIsKey(true);
Map<String, String> properties = Maps.newHashMap();
- properties.put(PropertyAnalyzer.PROPERTIES_BF_COLUMNS, "k1,v1");
+ properties.put(PropertyAnalyzer.PROPERTIES_BF_COLUMNS, "k1");
Set<String> bfColumns = PropertyAnalyzer.analyzeBloomFilterColumns(properties, columns);
- Assert.assertEquals(Sets.newHashSet("k1", "v1"), bfColumns);
+ Assert.assertEquals(Sets.newHashSet("k1"), bfColumns);
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org