[WIP](dynamic-table) support dynamic schema table #16335

eldenmoon · 2023-02-01T11:25:31Z

Proposed changes

Issue Number: close #16351

Dynamic schema table is a special type of table, it's schema change with loading procedure.Now we implemented this feature mainly for semi-structure data such as JSON, since JSON is schema self-described we could extract schema info from the original documents and inference the final type infomation.This speical table could reduce manual schema change operation and easily import semi-structure data and extends it's schema automatically.

Problem summary

Describe your changes.

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

There were too many comments to post at once. Showing the first 25 out of 40. Check the log or trigger a new build to see more.

be/src/exec/base_scanner.cpp

be/src/vec/columns/column_object.cpp

be/src/vec/columns/columns_common.cpp

be/src/vec/columns/subcolumn_tree.h

be/src/vec/core/accurate_comparison.h

be/src/vec/core/block.cpp

be/src/vec/data_types/get_least_supertype.cpp

github-actions

clang-tidy made some suggestions

be/src/vec/data_types/get_least_supertype.cpp

be/src/vec/exec/scan/vfile_scanner.cpp

hello-stephen · 2023-02-01T19:14:39Z

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 35.61 seconds
stream load tsv: 477 seconds loaded 74807831229 Bytes, about 149 MB/s
stream load json: 40 seconds loaded 2358488459 Bytes, about 56 MB/s
stream load orc: 69 seconds loaded 1101869774 Bytes, about 15 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230210125225_clickbench_pr_94072.html

eldenmoon · 2023-02-02T03:59:39Z

DOCS and regression test will be added in next PR

xiaokang

fe code reviewed

xiaokang · 2023-02-06T07:27:09Z

fe/fe-core/src/main/cup/sql_parser.cup

+        distribution, tblProperties, extProperties, tableComment, index, false);
+    :}
+    | KW_CREATE opt_external:isExternal KW_TABLE opt_if_not_exists:ifNotExists table_name:name
+            LPAREN column_definition_list:columns COMMA index_definition_list:indexes COMMA DOTDOTDOT RPAREN opt_engine:engineName


can we move ... just after column_definition_list to keep consistent

but it's a little bit difficult to get the bool value from column_definition_list, it's simple to just add a DOTDOTDOT token after column_definition_list

xiaokang · 2023-02-06T07:36:36Z

fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnDef.java

            }
-            if (defaultValue.isSet && defaultValue != DefaultValue.NULL_DEFAULT_VALUE) {
-                throw new AnalysisException("Array type column default value only support null");
+            if (defaultValue.isSet && defaultValue != DefaultValue.NULL_DEFAULT_VALUE


@cambyzju can you check the logic for default value?

it's ok for array type. null for NULL(Array), [] for NOT NULL(Array).

xiaokang · 2023-02-06T07:41:15Z

fe/fe-core/src/main/java/org/apache/doris/analysis/TupleDescriptor.java


    private float avgSerializedSize;  // in bytes; includes serialization overhead

+    private int tableId = -1;


what's purpose to assing tableId in TupleDescriptor

schema rpc need tableId to identify a specific table

xiaokang · 2023-02-06T07:45:04Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+        try {
+            if (!env.isMaster()) {
+                status.setStatusCode(TStatusCode.ILLEGAL_STATE);
+                status.addToErrorMsgs("retry rpc request to master.");


why not forward to master fe?

let the backend retry could be ok

xiaokang · 2023-02-06T07:48:35Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+                        }
+                        // ignore this column
+                        if (hasSameNameColumn) {
+                            continue;


How to process write if the type is not the same as the existed column?

the original will be casted to the existed

xiaokang · 2023-02-06T07:54:20Z

fe/fe-core/src/main/java/org/apache/doris/load/Load.java

            }
        }
+
+        // add a implict container column "__dynamic__" for dynamic columns


DORIS_DYNAMIC_COL

xiaokang · 2023-02-06T07:57:03Z

fe/fe-core/src/main/java/org/apache/doris/load/loadv2/LoadingTaskPlanner.java

            }
        }

+        if (table.isDynamicSchema()) {


add comment to explain the different to Load.initColumns()

for s3 load etc..

xiaokang · 2023-02-06T07:58:17Z

fe/fe-core/src/main/java/org/apache/doris/planner/StreamLoadPlanner.java

            }
        }

+        if (destTable.isDynamicSchema()) {


add comment to explain difference with Load.initColumns

gensrc/thrift/Types.thrift

xiaokang · 2023-02-06T08:04:10Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+
+                // prepare columnDefs
+                for (TColumnDef tColumnDef : addColumns) {
+                    if (request.isTypeConflictFree()) {


The name 'isTypeConflictFree' is not so intuitive. 'allowTypeConflict' may be better.

xiaokang

fe common code reviewed

xiaokang · 2023-02-06T08:28:41Z

be/src/olap/rowset/rowset_meta.h

    int64_t newest_write_timestamp() const { return _rowset_meta_pb.newest_write_timestamp(); }

    void set_tablet_schema(const TabletSchemaSPtr& tablet_schema) {
-        DCHECK(_schema == nullptr);


why delete DCHECK?

set_tablet_schema could override original _schema

xiaokang · 2023-02-06T08:41:14Z

be/src/vec/common/schema_util.cpp

+
+// send an empty add columns rpc, the rpc response will fill with base schema info
+// maybe we could seperate this rpc from add columns rpc
+Status send_fetch_full_base_schema_view_rpc(FullBaseSchemaView* schema_view) {


The name is misleading, since it's not just send rpc but also get result.

fetch could describe get result

xiaokang · 2023-02-06T08:45:55Z

be/src/olap/memtable.cpp

-    auto target_block = input_block->copy_block(_column_offset);
+    vectorized::Block target_block = *input_block;
+    // maybe rollup tablet, dynamic table's tablet need full columns
+    if (!_tablet_schema->is_dynamic_schema()) {


add comment for why not copy_block for dynamic schema

xiaokang · 2023-02-06T08:48:30Z

be/src/exec/base_scanner.cpp

                std::move(column_ptr), slot_desc->get_data_type_ptr(), slot_desc->col_name()));
    }

+    // handle dynamic generated columns


Does new scanners still use BaseScanner?

no new load scan will use vfile_scanner

xiaokang · 2023-02-06T08:51:05Z

be/src/vec/sink/vtablet_sink.cpp

    _rpc_timeout_ms = state->query_options().query_timeout * 1000;
    _timeout_watch.start();

-    _cur_mutable_block.reset(new vectorized::MutableBlock({_tuple_desc}));


why delete it

need handle dynamic block

xiaokang · 2023-02-06T09:08:56Z

be/src/vec/core/block.cpp

 }

 void MutableBlock::add_rows(const Block* block, size_t row_begin, size_t length) {
+    if (_type == BlockType::DYNAMIC) {


It may be more efficient to add UNLIKELY since most tables are without dynamic schema.

xiaokang · 2023-02-06T09:13:58Z

be/src/vec/io/var_int.h


 #include <iostream>

+#include "vec/common/string_buffer.hpp"


not used include?

just make compiler happy

xiaokang · 2023-02-06T09:29:16Z

be/src/vec/functions/if.cpp

    DataTypePtr get_return_type_impl(const DataTypes& arguments) const override {
-        return get_least_supertype({arguments[1], arguments[2]});
+        DataTypePtr type = nullptr;
+        get_least_supertype(DataTypes {arguments[1], arguments[2]}, &type);


why not keep return value?

type == null means encounter conflict

xiaokang · 2023-02-06T09:44:09Z

be/src/vec/columns/columns_common.h

+        return column.template index_impl<UInt16>(*data_uint16, limit);
+    } else if (auto* data_uint32 = detail::get_indexes_data<UInt32>(indexes)) {
+        return column.template index_impl<UInt32>(*data_uint32, limit);
+    } else if (auto* data_uint64 = detail::get_indexes_data<UInt64>(indexes)) {


Is the method suitable for Int8/16/32, Int/UInt128?

no index column could only be usigned integers

xiaokang · 2023-02-06T09:51:35Z

be/src/vec/data_types/get_least_supertype.h

  * Examples: least common supertype for UInt8, Int8 - Int16.
  * Examples: there is no least common supertype for Array(UInt8), Int8.
  */
-DataTypePtr get_least_supertype(const DataTypes& types);


should keep original version for backward compatability.

I think there is no need for backward compatability?

eldenmoon · 2023-02-01T11:29:54Z

gensrc/proto/types.proto

        FIXEDLENGTHOBJECT = 30;
        JSONB = 31;
        DECIMAL128I = 32;
+	    VARIANT = 33;


eldenmoon · 2023-02-07T09:27:58Z

fe/fe-core/src/main/cup/sql_parser.cup

+        distribution, tblProperties, extProperties, tableComment, index, false);
+    :}
+    | KW_CREATE opt_external:isExternal KW_TABLE opt_if_not_exists:ifNotExists table_name:name
+            LPAREN column_definition_list:columns COMMA index_definition_list:indexes COMMA DOTDOTDOT RPAREN opt_engine:engineName


but it's a little bit difficult to get the bool value from column_definition_list, it's simple to just add a DOTDOTDOT token after column_definition_list

eldenmoon · 2023-02-07T09:28:55Z

be/src/exec/base_scanner.cpp

                std::move(column_ptr), slot_desc->get_data_type_ptr(), slot_desc->col_name()));
    }

+    // handle dynamic generated columns


no new load scan will use vfile_scanner

eldenmoon · 2023-02-07T09:31:07Z

be/src/olap/memtable.cpp

-    auto target_block = input_block->copy_block(_column_offset);
+    vectorized::Block target_block = *input_block;
+    // maybe rollup tablet, dynamic table's tablet need full columns
+    if (!_tablet_schema->is_dynamic_schema()) {


eldenmoon · 2023-02-07T09:31:47Z

be/src/olap/rowset/rowset_meta.h

    int64_t newest_write_timestamp() const { return _rowset_meta_pb.newest_write_timestamp(); }

    void set_tablet_schema(const TabletSchemaSPtr& tablet_schema) {
-        DCHECK(_schema == nullptr);


set_tablet_schema could override original _schema

eldenmoon · 2023-02-07T09:53:57Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+        try {
+            if (!env.isMaster()) {
+                status.setStatusCode(TStatusCode.ILLEGAL_STATE);
+                status.addToErrorMsgs("retry rpc request to master.");


let the backend retry could be ok

eldenmoon · 2023-02-07T09:55:08Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+                        }
+                        // ignore this column
+                        if (hasSameNameColumn) {
+                            continue;


the original will be casted to the existed

eldenmoon · 2023-02-07T09:55:36Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+                }
+
+                if (!queryMode && !columnDefs.isEmpty()) {
+                    //3.create AddColumnsClause


gensrc/thrift/Types.thrift

eldenmoon · 2023-02-07T10:09:15Z

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

+
+                // prepare columnDefs
+                for (TColumnDef tColumnDef : addColumns) {
+                    if (request.isTypeConflictFree()) {


xiaokang

LGTM

github-actions · 2023-02-07T13:03:33Z

PR approved by anyone and no changes requested.

…e 'a.b.c'

…t code for ArrayType

…d columns

1. Load plan, inject variant type in the tuple descriptor end slot 2. VJsonScanner, parse json docs to dynamic column and extract columns 3. BaseScanner _materialize_dest_block 4. VBrokerScannode, adapt variant Block, since block schema may change during scan 5. VTabletSink, adapt variant Block, since block schema may change during sink

…st cases

* [fix](test) fix dynamic_table regression case * [improvement](inverted index) when excute compound predicate logic, check predicates which not apply by index

corret tablet should be used

…chema` property

…he#1331) * (improvement)[dynamic-table] support load in new_load_scan_node * [bugfix](thirdparty) patch simdjson to avoid conflict with odbc macro BOOL (apache#15223) fix conflit name BOOL in odbc sqltypes.h and simdjson element.h. Change BOOL to BOOLEAN in simdjson. Co-authored-by: Kang <kxiao.tiger@gmail.com>

…ache#1361) * [improvement](dynamic-table) refine some logic 1. support filter malformed json lines 2. refactor block align logic

morningman · 2023-02-11T05:36:19Z

gensrc/thrift/FrontendService.thrift

+    1: required Status.TStatus status
+    2: required i64 table_id
+    3: required list<Descriptors.TColumn> allColumns
+    4: required i32 schema_version


Use optional

morningman

LGTM, with 2 TODO:

use optional for all proto and thrift field
Add document for new configs

Issue Number: close apache#16351 Dynamic schema table is a special type of table, it's schema change with loading procedure.Now we implemented this feature mainly for semi-structure data such as JSON, since JSON is schema self-described we could extract schema info from the original documents and inference the final type infomation.This speical table could reduce manual schema change operation and easily import semi-structure data and extends it's schema automatically.

* [WIP](dynamic-table) support dynamic schema table (apache#16335) Issue Number: close apache#16351 Dynamic schema table is a special type of table, it's schema change with loading procedure.Now we implemented this feature mainly for semi-structure data such as JSON, since JSON is schema self-described we could extract schema info from the original documents and inference the final type infomation.This speical table could reduce manual schema change operation and easily import semi-structure data and extends it's schema automatically. * [improve](dynamic-table) change `addColumns` RPC interface fields from `required` to `optional` and and config doc (apache#16632) * [doc](dynamic-table) Add docs for dynamic-table (apache#16669) * [improve](dynamic table) refine SegmentWriter columns writer generate (apache#16816) * [improve](dynamic table) refine SegmentWriter columns writer generate ``` Dynamic Block consists of two parts, dynamic part of columns and static part of columns static dynamic | ----- | ------- | the static ones are original _tablet_schame columns the dynamic ones are auto generated and extended from file scan. ``` **We should only consisder to use Block info to generte columns when it's a dynamic table load procudure.** And seperate the static ones and dynamic ones * test * [typo](docs)fix dynamic Table version label (apache#16895) * [Feature](Dynamic schema table) step1 support schema change expression (apache#17494) 1. introduce a new type `VARIANT` to encapsulate dynamic generated columns for hidding the detail of types and names of newly generated columns 2. introduce a new expression `SchemaChangeExpr` for doing schema change for extensibility * Fix compile * [Bug](dynamic-table) Fix column alignment logic and support filtering null values when slot is not null Before this PR when encountering null values with some columns which is specified as `NOT NULL`, null values will not be filtered,thi behavior does not match with the original load behavior. Second column alignment logic has bug : ``` template <typename ColumnInserterFn> void align_variant_by_name_and_type(ColumnObject& dst, const ColumnObject& src, size_t row_cnt, ColumnInserterFn inserter) { CHECK(dst.is_finalized() && src.is_finalized()); // Use rows() here instead of size(), since size() will check_consistency // but we could not check_consistency since num_rows will be upgraded even // if src and dst is empty, we just increase the num_rows of dst and fill // num_rows of default values when meet new data size_t num_rows = dst.rows(); ``` --------- Co-authored-by: jiafeng.zhang <zhangjf1@gmail.com>

github-actions bot added area/load Issues or PRs related to all kinds of load area/planner Issues or PRs related to the query planner area/sql/function Issues or PRs related to the SQL functions area/vectorization labels Feb 1, 2023

eldenmoon force-pushed the dynamic-table branch from 994d87e to 9c1b0eb Compare February 1, 2023 12:11

github-actions bot reviewed Feb 1, 2023

View reviewed changes

eldenmoon force-pushed the dynamic-table branch from 260b12d to a32c1d5 Compare February 1, 2023 17:53

eldenmoon marked this pull request as ready for review February 2, 2023 04:17

eldenmoon force-pushed the dynamic-table branch from a32c1d5 to e2b9500 Compare February 2, 2023 07:58

xiaokang reviewed Feb 6, 2023

View reviewed changes

eldenmoon force-pushed the dynamic-table branch from e2b9500 to 0023cc1 Compare February 7, 2023 10:31

eldenmoon commented Feb 7, 2023

View reviewed changes

eldenmoon force-pushed the dynamic-table branch from 0023cc1 to 601f1a8 Compare February 7, 2023 10:32

xiaokang previously approved these changes Feb 7, 2023

View reviewed changes

github-actions bot added the reviewed label Feb 7, 2023

eldenmoon dismissed xiaokang’s stale review via a8cbdb3 February 7, 2023 13:04

eldenmoon force-pushed the dynamic-table branch 2 times, most recently from a8cbdb3 to 894478f Compare February 8, 2023 05:12

xiaokang previously approved these changes Feb 8, 2023

View reviewed changes

eldenmoon dismissed xiaokang’s stale review via d960979 February 9, 2023 12:21

eldenmoon force-pushed the dynamic-table branch 4 times, most recently from dbbedac to 0efbff0 Compare February 10, 2023 08:40

[feature-dynamic-table](variant) add variant type

0d8ce83

eldenmoon and others added 24 commits February 10, 2023 17:59

[feature-dynamic-table](syntax) support column which contains '.' lik…

eb01724

…e 'a.b.c'

[feature-dynamic-table](schemachange) support addColumns rpc and adap…

5239393

…t code for ArrayType

[feature-dynamic-table](vtablet_sink) support sink block with extende…

d3e5e29

…d columns

[feature-dynamic-table](vec/block) add names to MutableBlock

766a4e5

[]ture-dynamic-table](regression-test) add regression-test

6a362cc

[Fix](dynamic-table) update tablet schema during load and add more te…

f7037c8

…st cases

[fix] fix regression case (apache#894)

98de1ba

* [fix](test) fix dynamic_table regression case * [improvement](inverted index) when excute compound predicate logic, check predicates which not apply by index

[fix](dynamic-table) fix dynamic table in cloud mode

956eb17

corret tablet should be used

[feature-opt](dynamic-table) support show table displays `dynamic_s…

9bff039

…chema` property

[feature-dynamic-table](load) support dynamic table s3load

cf042b3

[regression-test](dynamic-table) fix dynamic table cases

5cd5c28

[compile](dynamic-table) fix compile in clang (apache#1333)

aa011d7

[improvement](dynamic-table) refine some logic and add more cases (ap…

c24163a

…ache#1361) * [improvement](dynamic-table) refine some logic 1. support filter malformed json lines 2. refactor block align logic

fix compile and format

4bcabc4

rename object_util to schema_util

f1b35a1

format

d11e928

remove regression case

9a4c789

fix license

22d17bb

format

156cf7d

fix FeNameFormatTest

f9a4bb7

modified by comments

5c07d34

rebase

37d7134

eldenmoon force-pushed the dynamic-table branch from 0efbff0 to 37d7134 Compare February 10, 2023 10:00

morningman reviewed Feb 11, 2023

View reviewed changes

morningman approved these changes Feb 11, 2023

View reviewed changes

morningman merged commit 37d1519 into apache:master Feb 11, 2023


		private float avgSerializedSize; // in bytes; includes serialization overhead

		private int tableId = -1;

[WIP](dynamic-table) support dynamic schema table #16335

[WIP](dynamic-table) support dynamic schema table #16335

Uh oh!

Conversation

eldenmoon commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

eldenmoon commented Feb 2, 2023

Uh oh!

xiaokang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eldenmoon commented Feb 1, 2023 •

edited

Loading

hello-stephen commented Feb 1, 2023 •

edited

Loading