[feature](schema change) unified schema change for parquet and orc reader #32873

AshinGau · 2024-03-27T02:02:03Z

Proposed changes

Following #25138, unified schema change interface for parquet and orc reader, and can be applied to other format readers as well.

Supported Type Changes

More type changes are supported:

ColumnTypeConverter

Unified schema change interface for all format readers:

First, read the data according to the column type of the file into source column;
Second, convert source column to the destination column with type planned by FE

PhysicalToLogicalConverter

Convert parquet physical column to logical column
In parquet document(https://github.com/apache/parquet-format/blob/master/LogicalTypes.md),
Logical or converted type is the data type of column, physical type is the stored type of column chunk. eg, decimal type can be stored as INT32, INT64, BYTE_ARRAY, FIXED_LENGTH_BYTE_ARRAY, so there is a convert process from physical type to logical type. In addition, schema change will bring about a change in logical type.

In previous implementations, physical and logical conversion were mixed together, resulting in severe code complexity and bloating.
PhysicalToLogicalConverter strips away the conversion of logical type, and reuse ColumnTypeConverter to resolve schema change, allowing parquet reader to only focus on the conversion of physical types.

Therefore, tow layers converters are designed:

First, read parquet data with the physical type
Second, convert physical type to logical type
Third, convert logical type to the final type planned by FE(schema change)

Ultimate performance optimization:

If process of (First => Second) is consistent, eg. from BYTE_ARRAY to string, no additional copies and conversions will be introduced;
If process of (Second => Third) is consistent, eg. from decimal(12, 4) to decimal(8, 2), no additional copies and conversions will be introduced;
Null map is share among all processes, no additional copies and conversions will be introduced in null map;
Only create one physical column in physical conversion, and reused in each loop;
Only create one logical column in logical conversion, and reused in each loop;
FIXED_LENGTH_BYTE_ARRAY is read as ColumnUInt8 instead of ColumnString, so the underlying decoder has no process to decode string and use memory copy to read the data as a whole, and the conversion has no need to resolve the Offsets in ColumnString.

Remaining Issues

Not support paimon timestamp type, because it has no logical or converted type, and is conflict with column type change from bigint to timestamp. Reference paimon issue: [format]Fix orc and parquet writer about timestamp not contains [local timezone] and [is_adjust_to_utc] paimon#2739,

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...

doris-robot · 2024-03-27T02:02:08Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

github-actions

clang-tidy made some suggestions

be/src/vec/exec/format/column_type_convert.cpp

be/src/vec/exec/format/column_type_convert.h

github-actions

clang-tidy made some suggestions

be/src/vec/exec/format/column_type_convert.cpp

be/src/vec/exec/format/column_type_convert.h

be/src/vec/exec/format/parquet/delta_bit_pack_decoder.h

be/src/vec/exec/format/parquet/parquet_column_convert.h

be/src/vec/exec/format/parquet/vparquet_column_reader.cpp

github-actions

clang-tidy made some suggestions

be/src/vec/exec/format/parquet/delta_bit_pack_decoder.h

be/src/vec/exec/format/parquet/parquet_column_convert.cpp

be/src/vec/exec/format/parquet/vparquet_column_reader.cpp

be/src/vec/exec/format/column_type_convert.h

morningman · 2024-04-07T05:56:51Z

be/src/vec/exec/format/column_type_convert.h

+            const char* startptr, const int buffer_size,
+            PrimitiveTypeTraits<TYPE_LARGEINT>::ColumnType::value_type* value) {
+        int64 cast_to_int = 0;
+        bool can_cast = safe_strto64(startptr, buffer_size, &cast_to_int);


still using 64 for largeint?

Use read_int_text_impl instead.

be/src/vec/exec/format/column_type_convert.h

be/src/vec/exec/format/column_type_convert.cpp

morningman · 2024-04-07T06:18:13Z

be/src/vec/data_types/data_type_struct.cpp

 }

 String DataTypeStruct::get_name_by_position(size_t i) const {
-    if (i == 0 || i > names.size()) {


Why removing this check?

This function is wrong, and there's no check like Block::get_by_position

morningman · 2024-04-07T06:21:12Z

be/src/vec/exec/format/orc/vorc_reader.cpp


+    PrimitiveType src_type = OrcReader::convert_to_doris_type(type).type;
+    if (src_type != primitive_type) {
+        if (!(is_string_type(src_type) && is_string_type(primitive_type))) {


Altering column type from string to varchar can still use push-down predicates.

be/src/vec/exec/format/parquet/parquet_column_convert.h

AshinGau · 2024-04-08T01:51:54Z

run buildall

AshinGau · 2024-04-08T08:27:42Z

run buildall

github-actions · 2024-04-08T15:38:10Z

PR approved by at least one committer and no changes requested.

github-actions · 2024-04-08T15:38:12Z

PR approved by anyone and no changes requested.

AshinGau · 2024-04-09T02:55:53Z

run buildall

github-actions

clang-tidy made some suggestions

be/src/vec/exec/format/parquet/parquet_column_convert.cpp

doris-robot · 2024-04-09T03:24:23Z

TeamCity be ut coverage result:
Function Coverage: 35.59% (8907/25028)
Line Coverage: 27.32% (73075/267483)
Region Coverage: 26.44% (37788/142898)
Branch Coverage: 23.21% (19256/82982)
Coverage Report: http://coverage.selectdb-in.cc/coverage/407efed4c318485f1b7a140a52be2f152f7fb32c_407efed4c318485f1b7a140a52be2f152f7fb32c/report/index.html

morningman

LGTM

github-actions · 2024-04-09T05:00:28Z

PR approved by at least one committer and no changes requested.

morningman · 2024-04-10T10:15:16Z

run buildall

doris-robot · 2024-04-10T10:41:37Z

TeamCity be ut coverage result:
Function Coverage: 35.63% (8911/25011)
Line Coverage: 27.33% (73148/267607)
Region Coverage: 26.46% (37828/142941)
Branch Coverage: 23.21% (19276/83038)
Coverage Report: http://coverage.selectdb-in.cc/coverage/6a3e4cb0b8088af4aee75de27968a8a6156b4a16_6a3e4cb0b8088af4aee75de27968a8a6156b4a16/report/index.html

AshinGau · 2024-04-10T11:09:24Z

run buildall

doris-robot · 2024-04-10T11:27:53Z

TeamCity be ut coverage result:
Function Coverage: 35.62% (8910/25011)
Line Coverage: 27.33% (73134/267607)
Region Coverage: 26.45% (37811/142941)
Branch Coverage: 23.21% (19269/83038)
Coverage Report: http://coverage.selectdb-in.cc/coverage/6c5b607f09b8e259b1650ee56aafb444f0360541_6c5b607f09b8e259b1650ee56aafb444f0360541/report/index.html

github-actions · 2024-04-10T13:15:24Z

PR approved by at least one committer and no changes requested.

kaka11chen

LGTM

…ader (#32873) Following #25138, unified schema change interface for parquet and orc reader, and can be applied to other format readers as well. Unified schema change interface for all format readers: - First, read the data according to the column type of the file into source column; - Second, convert source column to the destination column with type planned by FE.

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

…pache#33546) follow apache#32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

…ader (#32873) (#38408) ## Proposed changes bp #32873 Scenario: Reading a hive table after adding fields to a struct column Since there are still problems with reading tables in parquet and text formats on the master in this scenario, only tables in orc format are picked here and some cases are added.

…ader (apache#32873) (apache#38408) ## Proposed changes bp apache#32873 Scenario: Reading a hive table after adding fields to a struct column Since there are still problems with reading tables in parquet and text formats on the master in this scenario, only tables in orc format are picked here and some cases are added.

…ma change. (#47471) ### What problem does this PR solve? Related PR: #32873 Problem Summary: Explicitly defines the behavior of column type conversions. <img width="935" alt="image" src="https://github.com/user-attachments/assets/1e5afcf6-fbcf-4c36-b44e-82843feacb05" /> Special notes are as follows: `String => boolean`: In Parquet, only "false", "off", "no", "0", and an empty string ("") are considered false; otherwise, it is true. In Orc, a string can be parsed as a number, and if that number is 0, it is considered false; otherwise, it is true. If parsing the number fails, it results in null. Conversion between `Int/smallint/tinyint/bigint`: Unless the conversion can be perfectly represented, an error will be reported. For example: Bigint column => smallint column Reason: [INTERNAL_ERROR] Failed to cast value '9223372036854775807' to Nullable(Int16) column. Conversion between `Decimal`: Unless the conversion can be perfectly done, an error will be reported. `String => Int/smallint/tinyint/bigint`: It can be successfully converted to a number, and the number can be correctly stored. Otherwise, the result is null. `Int/smallint/tinyint/bigint => float`: The conversion is successful only if abs(number type) < 2^23. `Int/smallint/tinyint/bigint => double`: The conversion is successful only if abs(number type) < 2^52. `Decimal => Int/smallint/tinyint/bigint`: If the integer part of the decimal can be perfectly stored, only the integer part will be shown; otherwise, it will result in null. `Float => double`: Refer to the C++ static_cast<double>(float). `Decimal => float/double`: Attempt to store the approximate value. `Boolean => string`: The conversion will result in “TRUE” or “FALSE”. TODO: conversion to `char/varchar` type requires truncation.

…ma change. (apache#47471) ### What problem does this PR solve? Related PR: apache#32873 Problem Summary: Explicitly defines the behavior of column type conversions. <img width="935" alt="image" src="https://github.com/user-attachments/assets/1e5afcf6-fbcf-4c36-b44e-82843feacb05" /> Special notes are as follows: `String => boolean`: In Parquet, only "false", "off", "no", "0", and an empty string ("") are considered false; otherwise, it is true. In Orc, a string can be parsed as a number, and if that number is 0, it is considered false; otherwise, it is true. If parsing the number fails, it results in null. Conversion between `Int/smallint/tinyint/bigint`: Unless the conversion can be perfectly represented, an error will be reported. For example: Bigint column => smallint column Reason: [INTERNAL_ERROR] Failed to cast value '9223372036854775807' to Nullable(Int16) column. Conversion between `Decimal`: Unless the conversion can be perfectly done, an error will be reported. `String => Int/smallint/tinyint/bigint`: It can be successfully converted to a number, and the number can be correctly stored. Otherwise, the result is null. `Int/smallint/tinyint/bigint => float`: The conversion is successful only if abs(number type) < 2^23. `Int/smallint/tinyint/bigint => double`: The conversion is successful only if abs(number type) < 2^52. `Decimal => Int/smallint/tinyint/bigint`: If the integer part of the decimal can be perfectly stored, only the integer part will be shown; otherwise, it will result in null. `Float => double`: Refer to the C++ static_cast<double>(float). `Decimal => float/double`: Attempt to store the approximate value. `Boolean => string`: The conversion will result in “TRUE” or “FALSE”. TODO: conversion to `char/varchar` type requires truncation.

…ma change. (apache#47471) Related PR: apache#32873 Problem Summary: Explicitly defines the behavior of column type conversions. <img width="935" alt="image" src="https://github.com/user-attachments/assets/1e5afcf6-fbcf-4c36-b44e-82843feacb05" /> Special notes are as follows: `String => boolean`: In Parquet, only "false", "off", "no", "0", and an empty string ("") are considered false; otherwise, it is true. In Orc, a string can be parsed as a number, and if that number is 0, it is considered false; otherwise, it is true. If parsing the number fails, it results in null. Conversion between `Int/smallint/tinyint/bigint`: Unless the conversion can be perfectly represented, an error will be reported. For example: Bigint column => smallint column Reason: [INTERNAL_ERROR] Failed to cast value '9223372036854775807' to Nullable(Int16) column. Conversion between `Decimal`: Unless the conversion can be perfectly done, an error will be reported. `String => Int/smallint/tinyint/bigint`: It can be successfully converted to a number, and the number can be correctly stored. Otherwise, the result is null. `Int/smallint/tinyint/bigint => float`: The conversion is successful only if abs(number type) < 2^23. `Int/smallint/tinyint/bigint => double`: The conversion is successful only if abs(number type) < 2^52. `Decimal => Int/smallint/tinyint/bigint`: If the integer part of the decimal can be perfectly stored, only the integer part will be shown; otherwise, it will result in null. `Float => double`: Refer to the C++ static_cast<double>(float). `Decimal => float/double`: Attempt to store the approximate value. `Boolean => string`: The conversion will result in “TRUE” or “FALSE”. TODO: conversion to `char/varchar` type requires truncation.

github-actions bot reviewed Mar 27, 2024

View reviewed changes

AshinGau force-pushed the uni_change branch from 6eaa75c to 96bdc2c Compare April 2, 2024 01:45

github-actions bot reviewed Apr 2, 2024

View reviewed changes

AshinGau force-pushed the uni_change branch from 96bdc2c to fb9e5ed Compare April 7, 2024 03:41

github-actions bot reviewed Apr 7, 2024

View reviewed changes

morningman reviewed Apr 7, 2024

View reviewed changes

AshinGau force-pushed the uni_change branch from fb9e5ed to ed963bc Compare April 8, 2024 01:45

AshinGau force-pushed the uni_change branch from ed963bc to e013366 Compare April 8, 2024 08:20

morningman previously approved these changes Apr 8, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 8, 2024

github-actions bot added the reviewed label Apr 8, 2024

AshinGau dismissed morningman’s stale review via 407efed April 9, 2024 02:54

AshinGau force-pushed the uni_change branch from afd1139 to 407efed Compare April 9, 2024 02:54

github-actions bot removed the approved Indicates a PR has been approved by one committer. label Apr 9, 2024

github-actions bot reviewed Apr 9, 2024

View reviewed changes

be/src/vec/exec/format/parquet/parquet_column_convert.cpp Show resolved Hide resolved

be/src/vec/exec/format/parquet/parquet_column_convert.cpp Show resolved Hide resolved

morningman previously approved these changes Apr 9, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 9, 2024

AshinGau added 5 commits April 10, 2024 10:41

[feature](schema change)

42e2107

fix test

bdbd9f7

fix2

75edfb6

fix decimal

96822be

fix time to string

68eb80c

Merge branch 'master' into uni_change

6c5b607

morningman approved these changes Apr 10, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 10, 2024

kaka11chen approved these changes Apr 10, 2024

View reviewed changes

AshinGau merged commit 165b0a2 into apache:master Apr 10, 2024

AshinGau mentioned this pull request Apr 11, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ #33546

Merged

morningman pushed a commit that referenced this pull request Apr 12, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ (#…

607b333

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

yiguolei pushed a commit that referenced this pull request Apr 12, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ (#…

c690676

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

yiguolei pushed a commit that referenced this pull request Apr 13, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ (#…

24152e7

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

yiguolei pushed a commit that referenced this pull request Apr 13, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ (#…

7ea5e1d

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

yiguolei pushed a commit that referenced this pull request Apr 17, 2024

[fix](schema change) CastStringConverter is compiled failed in g++ (#…

ae68cca

…33546) follow #32873, CastStringConverter is compiled failed in g++ for uninitialized value, which is ok in clang:

hubgeter mentioned this pull request Jul 26, 2024

[feature](schema change) unified schema change for parquet and orc reader (#32873) #38408

Merged

wm1581066 added the dev/2.0.14-merged label Jul 31, 2024

hubgeter mentioned this pull request Feb 14, 2025

[enchement](schema change)Standardize the behavior after a table schema change. #47471

Merged

16 tasks

hubgeter mentioned this pull request Jun 23, 2025

[enchement](schema change)Standardize the behavior after a table schema change. (#47471) #52128

Merged

16 tasks

[feature](schema change) unified schema change for parquet and orc reader #32873

[feature](schema change) unified schema change for parquet and orc reader #32873

Uh oh!

Conversation

AshinGau commented Mar 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Supported Type Changes

ColumnTypeConverter

PhysicalToLogicalConverter

Ultimate performance optimization:

Remaining Issues

Further comments

Uh oh!

doris-robot commented Mar 27, 2024

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!

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!

morningman Apr 7, 2024

Choose a reason for hiding this comment

Uh oh!

AshinGau Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morningman Apr 7, 2024

Choose a reason for hiding this comment

Uh oh!

AshinGau Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

morningman Apr 7, 2024

Choose a reason for hiding this comment

Uh oh!

AshinGau Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AshinGau commented Apr 8, 2024

Uh oh!

AshinGau commented Apr 8, 2024

Uh oh!

github-actions bot commented Apr 8, 2024

Uh oh!

github-actions bot commented Apr 8, 2024

AshinGau commented Mar 27, 2024 •

edited

Loading