[fix](schema scanner)change schema_scanner::get_next_row to get_next_block #15718

Cai-Yao · 2023-01-09T03:11:12Z

Proposed changes

Issue Number: close #xxx

Problem summary

delete schema scanner::get_next_row，change to get_to_block

Checklist(Required)

Does it affect the original behavior:
- Yes
- No
- I don't know
Has unit tests been added:
- Yes
- No
- No Need
Has document been added or modified:
- Yes
- No
- No Need
Does it need to update dependencies:
- Yes
- No
Are there any changes that cannot be rolled back:
- Yes (If Yes, please explain WHY)
- No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-09T03:15:13Z

be/src/exec/schema_scanner/schema_tables_scanner.h


    virtual Status start(RuntimeState* state);
    virtual Status get_next_row(Tuple* tuple, MemPool* pool, bool* eos);
+    virtual Status get_next_block(vectorized::Block* block, bool* eos);


warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]

Suggested change

virtual Status get_next_block(vectorized::Block* block, bool* eos);

Status get_next_block(vectorized::Block* block, bool* eos) override;

github-actions · 2023-01-09T03:15:14Z

be/src/vec/exec/vschema_scan_node.cpp

-                if (!slot_desc->is_materialized()) {
-                    continue;
+                if (mem_reuse) {
+                    columns[i] = std::move(*block->get_by_position(i).column).mutate();


warning: std::move of the const expression has no effect; remove std::move() [performance-move-const-arg]

Suggested change

columns[i] = std::move(*block->get_by_position(i).column).mutate();

columns[i] = *block->get_by_position(i).column.mutate();

yiguolei · 2023-01-09T03:51:59Z

be/src/vec/exec/vschema_scan_node.cpp

-        DCHECK(block->rows() == 0);
+    if (_schema_scanner->type() == TSchemaTableType::SCH_TABLES) {
+        do {
+            bool mem_reuse = block->mem_reuse();


do not use mem reuse here, just call block->clear() to clear all columns in block and then reinsert columns.

yiguolei · 2023-01-09T03:52:59Z

be/src/exec/schema_scanner/schema_tables_scanner.cpp

                        sizeof(_s_tbls_columns) / sizeof(SchemaScanner::ColumnDesc)),
          _db_index(0),
-          _table_index(0) {}
+          _table_index(0) {


_table_index is useless

hello-stephen · 2023-01-09T05:10:56Z

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 35.13 seconds
load time: 504 seconds
storage size: 17122503506 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230129191323_clickbench_pr_86664.html

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-09T09:29:11Z

be/src/exec/schema_scanner/schema_tables_scanner.h

    SchemaTablesScanner();
    virtual ~SchemaTablesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-09T09:29:11Z

be/src/exec/schema_scanner/schema_tables_scanner.h

    virtual ~SchemaTablesScanner();

    virtual Status start(RuntimeState* state);
    virtual Status get_next_row(Tuple* tuple, MemPool* pool, bool* eos);


warning: 'get_next_row' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status get_next_row(Tuple* tuple, MemPool* pool, bool* eos); ^

be/src/exec/schema_scanner.h:85: overridden virtual function is here

virtual Status get_next_row(Tuple* tuple, MemPool* pool, bool* eos); ^

yiguolei · 2023-01-09T09:32:28Z

be/src/exec/schema_scanner/schema_tables_scanner.cpp

-
-Status SchemaTablesScanner::fill_one_row(vectorized::Block* block) {
-    const TTableStatus& tbl_status = _table_result.tables[_table_index];
+Status SchemaTablesScanner::fill_block_imp(vectorized::Block* block) {


fill_block_impl

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-11T06:32:47Z

be/src/exec/schema_scanner/schema_columns_scanner.h

 public:
    SchemaColumnsScanner();
    virtual ~SchemaColumnsScanner();
    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:48Z

be/src/exec/schema_scanner/schema_dummy_scanner.h

 public:
    SchemaDummyScanner();
    virtual ~SchemaDummyScanner();
    virtual Status start(RuntimeState* state = nullptr);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state = nullptr); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:48Z

be/src/exec/schema_scanner/schema_files_scanner.h

    SchemaFilesScanner();
    virtual ~SchemaFilesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:48Z

be/src/exec/schema_scanner/schema_partitions_scanner.h

    SchemaPartitionsScanner();
    virtual ~SchemaPartitionsScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:49Z

be/src/exec/schema_scanner/schema_schema_privileges_scanner.h

    SchemaSchemaPrivilegesScanner();
    virtual ~SchemaSchemaPrivilegesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:49Z

be/src/exec/schema_scanner/schema_schemata_scanner.h

    SchemaSchemataScanner();
    virtual ~SchemaSchemataScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:49Z

be/src/exec/schema_scanner/schema_table_privileges_scanner.h

    SchemaTablePrivilegesScanner();
    virtual ~SchemaTablePrivilegesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:49Z

be/src/exec/schema_scanner/schema_user_privileges_scanner.h

    SchemaUserPrivilegesScanner();
    virtual ~SchemaUserPrivilegesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:50Z

be/src/exec/schema_scanner/schema_variables_scanner.h

    SchemaVariablesScanner(TVarType::type type);
    virtual ~SchemaVariablesScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

github-actions · 2023-01-11T06:32:50Z

be/src/exec/schema_scanner/schema_views_scanner.h

    SchemaViewsScanner();
    virtual ~SchemaViewsScanner();

    virtual Status start(RuntimeState* state);


warning: 'start' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

virtual Status start(RuntimeState* state); ^

be/src/exec/schema_scanner.h:84: overridden virtual function is here

virtual Status start(RuntimeState* state); ^

yiguolei · 2023-01-12T07:24:28Z

be/src/exec/schema_scan_node.cpp

        }

-        RETURN_IF_ERROR(_schema_scanner->get_next_row(_src_tuple, _tuple_pool.get(), &scanner_eos));
+        // RETURN_IF_ERROR(_schema_scanner->get_next_row(_src_tuple, _tuple_pool.get(), &scanner_eos));


just remove the code

yiguolei · 2023-01-12T07:32:41Z

be/src/exec/schema_scanner/schema_rowsets_scanner.cpp

+        return Status::InternalError("input pointer is nullptr.");
+    }
+
+    *eos = true;


rowsets 这个表不一定可以一个block 一下子装进去的，可能有10000000个rowset

github-actions · 2023-01-12T12:02:57Z

clang-tidy review says "All clean, LGTM! 👍"

yiguolei · 2023-01-14T02:52:21Z

be/src/exec/schema_scanner.cpp

+    if (!block->has(slot_desc->col_name())) {
+        return Status::OK();
+    }
+    vectorized::IColumn* col_ptr = const_cast<vectorized::IColumn*>(


这里你不要直接使用裸指针，尽量使用 ColumnPtr 这种类似智能指针的结构

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-16T07:44:54Z

be/src/exec/schema_scanner.cpp

+        return Status::OK();
+    }
+    vectorized::MutableColumnPtr column_ptr =
+            std::move(*block->get_by_name(slot_desc->col_name()).column).mutate();


warning: std::move of the const expression has no effect; remove std::move() [performance-move-const-arg]

Suggested change

std::move(*block->get_by_name(slot_desc->col_name()).column).mutate();

*block->get_by_name(slot_desc->col_name()).column.mutate();

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-16T12:30:37Z

be/src/exec/schema_scanner.cpp

+        return Status::OK();
+    }
+    vectorized::MutableColumnPtr column_ptr =
+            std::move(*block->get_by_name(slot_desc->col_name()).column).assume_mutable();


warning: std::move of the const expression has no effect; remove std::move() [performance-move-const-arg]

Suggested change

std::move(*block->get_by_name(slot_desc->col_name()).column).assume_mutable();

*block->get_by_name(slot_desc->col_name()).column.assume_mutable();

github-actions

clang-tidy made some suggestions

be/src/vec/exec/vschema_scan_node.cpp

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-29T05:24:35Z

be/src/exec/schema_scanner.cpp

+        return Status::OK();
+    }
+    vectorized::MutableColumnPtr column_ptr =
+            std::move(*block->get_by_name(col_desc.name).column).assume_mutable();


warning: std::move of the const expression has no effect; remove std::move() [performance-move-const-arg]

Suggested change

std::move(*block->get_by_name(col_desc.name).column).assume_mutable();

*block->get_by_name(col_desc.name).column.assume_mutable();

github-actions

clang-tidy made some suggestions

github-actions · 2023-01-29T09:40:49Z

be/src/exec/schema_scanner.cpp

-        _columns = nullptr;
-    }
-}
+SchemaScanner::~SchemaScanner() {}


warning: use '= default' to define a trivial destructor [modernize-use-equals-default]

Suggested change

SchemaScanner::~SchemaScanner() {}

SchemaScanner::~SchemaScanner() = default;

github-actions · 2023-01-29T09:40:49Z

be/src/vec/exec/vschema_scan_node.cpp

-    delete[] reinterpret_cast<char*>(_dest_single_tuple);
-    _dest_single_tuple = nullptr;
-}
+VSchemaScanNode::~VSchemaScanNode() {}


warning: use '= default' to define a trivial destructor [modernize-use-equals-default]

Suggested change

VSchemaScanNode::~VSchemaScanNode() {}

VSchemaScanNode::~VSchemaScanNode() = default;

yiguolei

LGTM

…block (apache#15718)

github-actions bot added the area/vectorization label Jan 9, 2023

github-actions bot reviewed Jan 9, 2023

View reviewed changes

yiguolei reviewed Jan 9, 2023

View reviewed changes

github-actions bot reviewed Jan 9, 2023

View reviewed changes

yiguolei reviewed Jan 9, 2023

View reviewed changes

github-actions bot added the kind/test label Jan 11, 2023

github-actions bot reviewed Jan 11, 2023

View reviewed changes

yiguolei reviewed Jan 12, 2023

View reviewed changes

Cai-Yao force-pushed the cwk-dev branch from 0a93368 to 775abe5 Compare January 12, 2023 09:14

yiguolei reviewed Jan 14, 2023

View reviewed changes

github-actions bot reviewed Jan 16, 2023

View reviewed changes

Cai-Yao force-pushed the cwk-dev branch from 160f6ee to 0b6954f Compare January 16, 2023 12:24

github-actions bot reviewed Jan 16, 2023

View reviewed changes

Cai-Yao force-pushed the cwk-dev branch from 1a31805 to e21c708 Compare January 17, 2023 12:28

Cai-Yao added 12 commits January 28, 2023 10:15

add schema_tables_scanner get_next_block

70856aa

clang-format

bb01d2b

clang-format 2

5f54c42

fix

f1d4f4e

change schema_backends_scanner

471102b

modify charsets, collations

8dffcce

fix bug and modify columns

565268a

modify left schema scanner

a9a356a

add regression-test

3d5a7d2

fix rowsets_scanner

8249438

fix

945b008

fix again

941e410

Cai-Yao added 6 commits January 28, 2023 10:30

assume_mutable replace mutable

296b24d

fix regression test error and add sys tables test

f0a04e3

fix code format

b3a4381

add License

849f04e

fix regression test

8036bbb

fix regression again

52e59cf

Cai-Yao force-pushed the cwk-dev branch from 9528bf3 to 52e59cf Compare January 28, 2023 02:39

Cai-Yao added 2 commits January 28, 2023 11:38

fix format

d1942e5

fix rebase bug

6978825

github-actions bot reviewed Jan 28, 2023

View reviewed changes

be/src/vec/exec/vschema_scan_node.cpp Show resolved Hide resolved

change tuple_desc to column_desc

008dfda

github-actions bot reviewed Jan 29, 2023

View reviewed changes

refactor

908c179

github-actions bot reviewed Jan 29, 2023

View reviewed changes

Cai-Yao added 2 commits January 29, 2023 18:05

fix

e1a1146

fix be ut bug

4f84de5

yiguolei approved these changes Jan 30, 2023

View reviewed changes

yiguolei merged commit 69e748b into apache:master Jan 30, 2023

dutyu pushed a commit to dutyu/doris that referenced this pull request Feb 1, 2023

[fix](schema scanner)change schema_scanner::get_next_row to get_next_…

cf6a18f

…block (apache#15718)

dutyu pushed a commit to dutyu/doris that referenced this pull request Feb 6, 2023

[fix](schema scanner)change schema_scanner::get_next_row to get_next_…

1daae8a

…block (apache#15718)

Cai-Yao mentioned this pull request Mar 6, 2023

[Enhencement](schema scanner)change schema_scanner::get_next_row to get_next_block #17479

Closed

5 tasks

	virtual Status get_next_block(vectorized::Block* block, bool* eos);
	Status get_next_block(vectorized::Block* block, bool* eos) override;

	columns[i] = std::move(*block->get_by_position(i).column).mutate();
	columns[i] = *block->get_by_position(i).column.mutate();

	std::move(*block->get_by_name(slot_desc->col_name()).column).mutate();
	*block->get_by_name(slot_desc->col_name()).column.mutate();

	std::move(*block->get_by_name(col_desc.name).column).assume_mutable();
	*block->get_by_name(col_desc.name).column.assume_mutable();

	SchemaScanner::~SchemaScanner() {}
	SchemaScanner::~SchemaScanner() = default;

	VSchemaScanNode::~VSchemaScanNode() {}
	VSchemaScanNode::~VSchemaScanNode() = default;

[fix](schema scanner)change schema_scanner::get_next_row to get_next_block #15718

[fix](schema scanner)change schema_scanner::get_next_row to get_next_block #15718

Uh oh!

Conversation

Cai-Yao commented Jan 9, 2023

Proposed changes

Problem summary

Checklist(Required)

Further comments

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

yiguolei Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

yiguolei Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

yiguolei Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

yiguolei Jan 12, 2023

Choose a reason for hiding this comment

Uh oh!

yiguolei Jan 12, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 12, 2023

hello-stephen commented Jan 9, 2023 •

edited

Loading