-
Notifications
You must be signed in to change notification settings - Fork 186
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
[Coral-Trino] Migrate cast transformation from RelNode layer to SqlNode layer #491
Changes from all commits
16e7110
4acd5ba
bb8fffb
e540e4d
fb7a309
45da938
686e4e3
af01f97
bdd7907
1fb5179
a1f126f
d1bc9c4
d553285
27aa188
0fcd7d6
83f2357
82bf185
0884fac
71172d9
dfaec55
ccfdcfd
4356109
2a2d1ea
9f2bade
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/** | ||
* Copyright 2017-2023 LinkedIn Corporation. All rights reserved. | ||
KevinGe00 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Copyright 2017-2024 LinkedIn Corporation. All rights reserved. | ||
* Licensed under the BSD-2 Clause license. | ||
* See LICENSE in the project root for license information. | ||
*/ | ||
|
@@ -19,7 +19,6 @@ | |
import org.apache.calcite.rel.rel2sql.RelToSqlConverter; | ||
import org.apache.calcite.rel.type.RelDataType; | ||
import org.apache.calcite.rel.type.RelDataTypeField; | ||
import org.apache.calcite.rex.RexCall; | ||
import org.apache.calcite.rex.RexFieldAccess; | ||
import org.apache.calcite.rex.RexLiteral; | ||
import org.apache.calcite.rex.RexNode; | ||
|
@@ -35,7 +34,6 @@ | |
import com.linkedin.coral.common.functions.FunctionFieldReferenceOperator; | ||
|
||
import static com.google.common.base.Preconditions.*; | ||
import static com.linkedin.coral.trino.rel2trino.Calcite2TrinoUDFConverter.convertRel; | ||
import static com.linkedin.coral.trino.rel2trino.CoralTrinoConfigKeys.*; | ||
|
||
|
||
|
@@ -48,7 +46,7 @@ public class RelToTrinoConverter extends RelToSqlConverter { | |
* wrapping in {@link RelToTrinoConverter#visit(Uncollect)} | ||
* (2) Some internally registered UDFs which should not be converted, like `to_date`. | ||
* If the value of key {@link CoralTrinoConfigKeys#AVOID_TRANSFORM_TO_DATE_UDF} is set to true, we don't transform `to_date` UDF | ||
* in {@link com.linkedin.coral.trino.rel2trino.Calcite2TrinoUDFConverter.TrinoRexConverter#visitCall(RexCall)} | ||
* in {@link com.linkedin.coral.trino.rel2trino.transformers.ToDateOperatorTransformer} | ||
* (3) We need to adjust the return type for some functions using cast, since the converted Trino function's return type is not | ||
* aligned with the Hive function's return type. For example, if the value of key {@link CoralTrinoConfigKeys#CAST_DATEADD_TO_STRING} | ||
* is set to true, we would cast the converted RexCall to `varchar` type (date_add(xxx) -> cast(date_add(xxx) as varchar)) | ||
|
@@ -84,8 +82,7 @@ public RelToTrinoConverter(HiveMetastoreClient mscClient, Map<String, Boolean> c | |
* @return SQL string | ||
*/ | ||
public String convert(RelNode relNode) { | ||
RelNode rel = convertRel(relNode, configs); | ||
SqlNode sqlNode = convertToSqlNode(rel); | ||
SqlNode sqlNode = convertToSqlNode(relNode); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you ignore the configs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aastha25 could you verify this change is safe? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be safe to remove. |
||
|
||
SqlNode sqlNodeWithRelDataTypeDerivedConversions = | ||
sqlNode.accept(new DataTypeDerivedSqlCallConverter(_hiveMetastoreClient, sqlNode)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the ordering relevant here? Generally we add the new transformer at the end. But in this case,
CastOperatorTransformer
is appended beforeUnionSqlCallTransformer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Order shouldn't matter here since
UnionSqlCallTransformer
doesn't create any types of cast thatCastOperatorTransformer
would need to transform.