You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "lwz9103 (via GitHub)" <gi...@apache.org> on 2024/12/17 08:35:46 UTC
[PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
lwz9103 opened a new pull request, #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256
## What changes were proposed in this pull request?
(Fixes: \#8253)
## How was this patch tested?
unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2547808110
https://github.com/apache/incubator-gluten/issues/8253
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1893370364
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
Review Comment:
use chassert
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1893370717
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
Review Comment:
use checkAndGetDataType
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "lwz9103 (via GitHub)" <gi...@apache.org>.
lwz9103 commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1895338962
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -418,12 +418,11 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
}
DB::DataTypePtr elem_type;
- std::tie(elem_type, std::ignore) = LiteralParser::parse(options[0].literal());
- elem_type = wrapNullableType(nullable, elem_type);
-
- DB::MutableColumnPtr elem_column = elem_type->createColumn();
- elem_column->reserve(options_len);
- for (int i = 0; i < options_len; ++i)
+ std::vector<std::pair<DB::DataTypePtr, DB::Field>> options_type_and_field;
+ auto first_option = LiteralParser::parse(options[0].literal());
+ elem_type = wrapNullableType(nullable, first_option.first);
+ options_type_and_field.emplace_back(first_option);
Review Comment:
thanks, fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2559043938
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2550286461
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2547808576
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1893373532
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
+ DataTypes new_types;
+ for (int i = 0; i < tuple_type->getElements().size(); ++i)
Review Comment:
better to refactor it with `castColumn`. You could delete dozens of codes after refactor.
1. Given the type of first argument in `in` expression
2. Given the column of second argument in `in` expression
3. Cast the column of second argument to the type of first argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "lwz9103 (via GitHub)" <gi...@apache.org>.
lwz9103 commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1895340232
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
+ DataTypes new_types;
+ for (int i = 0; i < tuple_type->getElements().size(); ++i)
Review Comment:
we can directly use the first tuple type as the type of the second to avoid nullable mismatch.
1. Spark guarantees that the types of tuples in the 'in' filter are completely consistent. See org.apache.spark.sql.types.DataType#equalsStructurally
2. Additionally, the mapping from Spark types to ClickHouse types is one-to-one, See TypeParser.cpp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "lwz9103 (via GitHub)" <gi...@apache.org>.
lwz9103 commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1895340232
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
+ DataTypes new_types;
+ for (int i = 0; i < tuple_type->getElements().size(); ++i)
Review Comment:
we can directly use the first tuple type as the type of the tuple to avoid nullable mismatch.
1. Spark guarantees that the types of tuples in the 'in' filter are completely consistent. See org.apache.spark.sql.types.DataType#equalsStructurally
2. Additionally, the mapping from Spark types to ClickHouse types is one-to-one, See TypeParser.cpp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "baibaichen (via GitHub)" <gi...@apache.org>.
baibaichen merged PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2559346721
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#issuecomment-2556136320
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "lwz9103 (via GitHub)" <gi...@apache.org>.
lwz9103 commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1895339222
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
Review Comment:
fixed
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
+ assert(tuple_type->getElements().size() == result_type->getElements().size());
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "lwz9103 (via GitHub)" <gi...@apache.org>.
lwz9103 commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1895339483
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1893367551
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -418,12 +418,11 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
}
DB::DataTypePtr elem_type;
- std::tie(elem_type, std::ignore) = LiteralParser::parse(options[0].literal());
- elem_type = wrapNullableType(nullable, elem_type);
-
- DB::MutableColumnPtr elem_column = elem_type->createColumn();
- elem_column->reserve(options_len);
- for (int i = 0; i < options_len; ++i)
+ std::vector<std::pair<DB::DataTypePtr, DB::Field>> options_type_and_field;
+ auto first_option = LiteralParser::parse(options[0].literal());
+ elem_type = wrapNullableType(nullable, first_option.first);
+ options_type_and_field.emplace_back(first_option);
Review Comment:
better add std::move
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8253][CH] Fix cast failed when in-filter with tuple values [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li commented on code in PR #8256:
URL: https://github.com/apache/incubator-gluten/pull/8256#discussion_r1893371153
##########
cpp-ch/local-engine/Parser/ExpressionParser.cpp:
##########
@@ -433,8 +432,31 @@ const ActionsDAG::Node * ExpressionParser::parseExpression(ActionsDAG & actions_
"SingularOrList options type mismatch:{} and {}",
elem_type->getName(),
option_type->getName());
+ options_type_and_field.emplace_back(type_and_field);
+ }
- elem_column->insert(type_and_field.second);
+ // check tuple internal types
+ if (isTuple(elem_type) && isTuple(args[0]->result_type))
+ {
+ // align tuple inner types with nullable
+ auto tuple_type = std::static_pointer_cast<const DB::DataTypeTuple>(elem_type);
+ auto result_type = std::static_pointer_cast<const DB::DataTypeTuple>(args[0]->result_type);
Review Comment:
as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org