Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YQL-18053: Add block implementation for AsStruct callable #3485

Merged
merged 7 commits into from
Apr 22, 2024

Conversation

igormunkin
Copy link
Collaborator

@igormunkin igormunkin commented Apr 4, 2024

This patchset introduces block implementation for AsStruct callable. The semantics of this callable differs from AsTuple callable only in the resulting type of the object being processed (structure vs tuple). Hence, the first patch of the series turns Nth callable block implementation into generic AsContainer helper, being used by both AsTuple and AsStruct runtime node emitter.

Apart from the type annotation and runtime node emitter, there is a patch in the series that makes CollectBlockRewrites handle the rewrites related to the AsStruct arguments. AsStruct arguments (i.e. the members of the resulting structure object) are literal tuples, that don't propagate their child rewrites. As a result, if there is a block rewrite occurred in the expression graph below any of these lists, it takes no effect above this "list barrier". Hence, all AsStruct children should be traversed separately to either wrap the complete expressions, supported by the block engine, with AsScalar callable, or apply the necessary rewrites. If none of the conditions above are met, the block rewrite for AsStruct node is aborted, since one of the children being proceseed is rewritten neither to block, nor scalar, ergo, violating BlockAsStruct type requirements.

Changelog category

  • Improvement

Copy link

github-actions bot commented Apr 4, 2024

2024-04-04 20:41:45 UTC Pre-commit check for 4521f39 has started.
2024-04-04 20:41:47 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-04 20:56:17 UTC Build successful.

Copy link

github-actions bot commented Apr 4, 2024

2024-04-04 20:41:57 UTC Pre-commit check for 4521f39 has started.
2024-04-04 20:41:59 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-04 20:58:21 UTC Build successful.
2024-04-04 21:00:17 UTC Tests are running...
🔴 2024-04-04 22:40:52 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13677 13374 0 67 223 13

Copy link

github-actions bot commented Apr 4, 2024

2024-04-04 20:41:59 UTC Pre-commit check for 4521f39 has started.
2024-04-04 20:42:01 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-04 20:58:08 UTC Build successful.
2024-04-04 21:00:05 UTC Tests are running...
🔴 2024-04-04 22:51:31 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
68614 57348 0 114 11128 24

@@ -5468,7 +5468,7 @@ bool CollectBlockRewrites(const TMultiExprType* multiInputType, bool keepInputCo

TExprNode::TListType funcArgs;
std::string_view arrowFunctionName;
if (node->IsList() || node->IsCallable({"And", "Or", "Xor", "Not", "Coalesce", "If", "Just", "Nth", "ToPg", "FromPg", "PgResolvedCall", "PgResolvedOp"}))
if (node->IsList() || node->IsCallable({"And", "Or", "Xor", "Not", "Coalesce", "If", "Just", "Member", "Nth", "ToPg", "FromPg", "PgResolvedCall", "PgResolvedOp"}))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AsStruct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revision was irrelevant. Force-pushed.

@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from 965f431 to 66c3d24 Compare April 16, 2024 11:18
Copy link

github-actions bot commented Apr 16, 2024

2024-04-16 11:21:35 UTC Pre-commit check for b49a28d has started.
2024-04-16 11:21:44 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-16 11:56:06 UTC Build successful.

Copy link

github-actions bot commented Apr 16, 2024

2024-04-16 11:21:42 UTC Pre-commit check for b49a28d has started.
2024-04-16 11:21:45 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-16 11:43:03 UTC Build successful.
2024-04-16 11:45:00 UTC Tests are running...
🔴 2024-04-16 13:26:09 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13785 13469 0 62 221 33

Copy link

github-actions bot commented Apr 16, 2024

2024-04-16 11:23:16 UTC Pre-commit check for b49a28d has started.
2024-04-16 11:23:19 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-16 11:43:40 UTC Build successful.
2024-04-16 11:45:28 UTC Tests are running...
🔴 2024-04-16 13:33:23 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
61273 50585 0 422 10238 28

@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from 66c3d24 to 80ceb23 Compare April 17, 2024 22:02
Copy link

github-actions bot commented Apr 17, 2024

2024-04-17 22:05:42 UTC Pre-commit check for 098725a has started.
2024-04-17 22:05:44 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-17 22:20:27 UTC Build successful.

Copy link

github-actions bot commented Apr 17, 2024

2024-04-17 22:05:49 UTC Pre-commit check for 098725a has started.
2024-04-17 22:05:51 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-17 22:23:39 UTC Build successful.
2024-04-17 22:25:36 UTC Tests are running...
🔴 2024-04-18 00:41:35 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13768 13481 0 64 194 29

Copy link

github-actions bot commented Apr 17, 2024

2024-04-17 22:05:52 UTC Pre-commit check for 098725a has started.
2024-04-17 22:05:55 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-17 22:22:15 UTC Build successful.
2024-04-17 22:24:07 UTC Tests are running...
🔴 2024-04-18 00:26:10 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
69205 57453 0 488 11240 24

@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from 80ceb23 to 568b6c8 Compare April 18, 2024 08:41
Copy link

github-actions bot commented Apr 18, 2024

2024-04-18 08:42:48 UTC Pre-commit check for c631662 has started.
2024-04-18 08:42:50 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-18 09:00:09 UTC Build successful.

Copy link

github-actions bot commented Apr 18, 2024

2024-04-18 08:44:35 UTC Pre-commit check for c631662 has started.
2024-04-18 08:44:38 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-18 08:59:15 UTC Build successful.
2024-04-18 09:01:02 UTC Tests are running...
🔴 2024-04-18 11:14:22 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13769 13456 0 57 229 27

Copy link

github-actions bot commented Apr 18, 2024

2024-04-18 08:44:42 UTC Pre-commit check for c631662 has started.
2024-04-18 08:44:43 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-18 09:01:18 UTC Build successful.
2024-04-18 09:03:07 UTC Tests are running...
🔴 2024-04-18 11:08:56 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
69202 57806 0 90 11275 31

Copy link

github-actions bot commented Apr 19, 2024

2024-04-19 17:36:32 UTC Pre-commit check for 1e9b41f has started.
2024-04-19 17:36:35 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-19 17:50:12 UTC Build successful.

Copy link

github-actions bot commented Apr 19, 2024

2024-04-19 17:36:45 UTC Pre-commit check for 1e9b41f has started.
2024-04-19 17:36:47 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-19 17:53:20 UTC Build successful.
2024-04-19 17:55:18 UTC Tests are running...
🔴 2024-04-19 19:45:11 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13797 13618 0 47 116 16

Copy link

github-actions bot commented Apr 19, 2024

2024-04-19 17:36:54 UTC Pre-commit check for 1e9b41f has started.
2024-04-19 17:36:56 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-19 17:51:22 UTC Build successful.
2024-04-19 17:53:13 UTC Tests are running...
🔴 2024-04-19 19:49:01 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72247 59045 0 80 13107 15

@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from 3fa0116 to f8dc908 Compare April 20, 2024 18:04
Copy link

github-actions bot commented Apr 20, 2024

2024-04-20 18:08:00 UTC Pre-commit check for 4047b05 has started.
2024-04-20 18:08:03 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-20 18:23:00 UTC Build successful.
2024-04-20 18:24:54 UTC Tests are running...
🔴 2024-04-20 20:13:08 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13795 13610 0 49 121 15

Copy link

github-actions bot commented Apr 20, 2024

2024-04-20 18:08:03 UTC Pre-commit check for 4047b05 has started.
2024-04-20 18:08:05 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-20 18:23:03 UTC Build successful.
2024-04-20 18:24:55 UTC Tests are running...
🔴 2024-04-20 20:19:50 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72248 59103 0 22 13106 17

<AsStruct> arguments (i.e. the members of the resulting structure
object) are literal tuples, that don't propagate their child rewrites.
As a result, if there is a block rewrite occurred in the expression
graph below any of these lists, it takes no effect above this "list
barrier". Hence, all <AsStruct> children should be traversed separately
to either wrap the complete expressions, supported by the block engine,
with <AsScalar> callable, or apply the necessary rewrites. If none of
the conditions above are met, the block rewrite for <AsStruct> node is
aborted, since one of the children being proceseed is rewritten neither
to block, nor scalar, ergo, violating <BlockAsStruct> type requirements.
Since Tuple type support in block execution is implemented via Arrow
Struct type, all the machinery is renamed by s/AsTuple/AsContainer/ rule
and will be used for Struct type support either.
@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from f8dc908 to 3383779 Compare April 21, 2024 18:37
Copy link

github-actions bot commented Apr 21, 2024

2024-04-21 18:41:11 UTC Pre-commit check for d824639 has started.
2024-04-21 18:41:13 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-21 18:44:44 UTC Build successful.
2024-04-21 18:46:40 UTC Tests are running...
🔴 2024-04-21 21:08:20 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72250 59109 0 18 13108 15

Copy link

github-actions bot commented Apr 21, 2024

2024-04-21 18:41:17 UTC Pre-commit check for d824639 has started.
2024-04-21 18:41:18 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-21 18:45:10 UTC Build successful.
2024-04-21 18:47:05 UTC Tests are running...
🔴 2024-04-21 20:50:07 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13797 13614 0 49 117 17

Copy link

github-actions bot commented Apr 21, 2024

2024-04-21 18:41:23 UTC Pre-commit check for d824639 has started.
2024-04-21 18:41:25 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-21 18:45:02 UTC Build successful.

@igormunkin igormunkin force-pushed the YQL-18053-block-struct branch from 3383779 to e40c78f Compare April 22, 2024 05:06
Copy link

github-actions bot commented Apr 22, 2024

2024-04-22 05:10:22 UTC Pre-commit check for 5ff0655 has started.
2024-04-22 05:10:24 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-04-22 05:13:44 UTC Build successful.

Copy link

github-actions bot commented Apr 22, 2024

2024-04-22 05:10:26 UTC Pre-commit check for 5ff0655 has started.
2024-04-22 05:10:29 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-04-22 05:13:36 UTC Build successful.
2024-04-22 05:15:26 UTC Tests are running...
🔴 2024-04-22 07:08:46 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72251 59111 0 17 13107 16

Copy link

github-actions bot commented Apr 22, 2024

2024-04-22 05:10:29 UTC Pre-commit check for 5ff0655 has started.
2024-04-22 05:10:31 UTC Build linux-x86_64-release-asan is running...
🟢 2024-04-22 05:13:13 UTC Build successful.
2024-04-22 05:15:04 UTC Tests are running...
🔴 2024-04-22 07:03:19 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13797 13615 0 45 121 16

@igormunkin igormunkin merged commit f1bf06c into ydb-platform:main Apr 22, 2024
3 of 5 checks passed
nepal added a commit that referenced this pull request Apr 22, 2024
@igormunkin igormunkin deleted the YQL-18053-block-struct branch April 22, 2024 14:10
igormunkin added a commit to igormunkin/ydb that referenced this pull request Apr 23, 2024
…b-platform#3485)"

This reverts commit f1bf06c since there
are several broken tests, that were not fixed in ydb-platform#3485.
igormunkin added a commit to igormunkin/ydb that referenced this pull request Apr 25, 2024
This check is already implemented in <TBlockFuncNode::DoCalculate>, so
if the type annotation is invalid or a given Arrow Datum doesn't match
the annotated type, the debug build will hit the assertion. This patch
makes the Datum type check in <TWideFromBlocksWrapper::TState::Get>
as strict as in <TBlockFuncNode::DoCalculate>.

Relates to ydb-platform#3485
igormunkin added a commit to igormunkin/ydb that referenced this pull request May 2, 2024
This check is already implemented in <TBlockFuncNode::DoCalculate>, so
if the type annotation is invalid or a given Arrow Datum doesn't match
the annotated type, the debug build will hit the assertion. This patch
makes the Datum type check in <TWideFromBlocksWrapper::TState::Get>
as strict as in <TBlockFuncNode::DoCalculate>.

Relates to ydb-platform#3485
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants