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.

Sign up for GitHub

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

Jump to bottom

[Coral-Trino] Migrate cast transformation from RelNode layer to SqlNode layer #491

Merged
merged 24 commits into from
Apr 11, 2024
Merged

[Coral-Trino] Migrate cast transformation from RelNode layer to SqlNode layer #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
16e7110
Initial commit for migrating Cast
aastha25 Jul 18, 2023
4acd5ba
add type derivation changes
aastha25 Jul 26, 2023
bb8fffb
type derivation enhance and test debug
aastha25 Jul 28, 2023
e540e4d
merge master
KevinGe00 Jan 24, 2024
fb7a309
remove testing files
KevinGe00 Jan 24, 2024
45da938
spotlessapply + comment out failing unit test
KevinGe00 Jan 24, 2024
686e4e3
Merge branch 'master' into aastha25-movetrinocast
KevinGe00 Jan 24, 2024
af01f97
update unit tests
KevinGe00 Jan 27, 2024
bdd7907
fix regression by TypeDerivationUtil on casts with join on simple ali…
KevinGe00 Jan 27, 2024
1fb5179
generalize case of casting with join on simple aliases
KevinGe00 Jan 29, 2024
a1f126f
remove extraneous input to dummy select
KevinGe00 Feb 1, 2024
d1bc9c4
temporary removal of type util postprocessor for i-test
KevinGe00 Feb 1, 2024
d553285
revert
KevinGe00 Feb 1, 2024
27aa188
spotless checks
KevinGe00 Feb 16, 2024
0fcd7d6
update calcite version for linkedin-calcite #98/#99
KevinGe00 Feb 16, 2024
83f2357
remove Calcite2TrinoUDFConverter
KevinGe00 Feb 18, 2024
82bf185
clean up tests
KevinGe00 Feb 18, 2024
0884fac
unused imports
KevinGe00 Feb 19, 2024
71172d9
spotless
KevinGe00 Feb 20, 2024
dfaec55
improve javadoc/documentation
KevinGe00 Apr 8, 2024
ccfdcfd
fix javadoc
KevinGe00 Apr 8, 2024
4356109
improve javadoc
KevinGe00 Apr 9, 2024
2a2d1ea
grammar
KevinGe00 Apr 9, 2024
9f2bade
refactor + clean up
KevinGe00 Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coral-common/src/main/java/com/linkedin/coral/common/ToRelConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2017-2023 LinkedIn Corporation. All rights reserved.
* Copyright 2017-2024 LinkedIn Corporation. All rights reserved.
* Licensed under the BSD-2 Clause license.
* See LICENSE in the project root for license information.
*/
Expand Down
7 changes: 6 additions & 1 deletion coral-common/src/main/java/com/linkedin/coral/common/utils/TypeDerivationUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2023 LinkedIn Corporation. All rights reserved.
* Copyright 2023-2024 LinkedIn Corporation. All rights reserved.
* Licensed under the BSD-2 Clause license.
* See LICENSE in the project root for license information.
*/
Expand Down Expand Up @@ -59,6 +59,11 @@ public RelDataType getRelDataType(SqlNode sqlNode) {
throw new RuntimeException("SqlValidator does not exist to derive the RelDataType for SqlNode: " + sqlNode);
}

RelDataType cachedDataType = sqlValidator.getValidatedNodeTypeIfKnown(sqlNode);
if (cachedDataType != null) {
return cachedDataType;
}

for (SqlSelect topSqlSelectNode : topSelectNodes) {
final SqlSelect dummySqlSelect = new SqlSelect(topSqlSelectNode.getParserPosition(), null,
SqlNodeList.of(sqlNode), topSqlSelectNode.getFrom(), topSqlSelectNode.getWhere(), topSqlSelectNode.getGroup(),
Expand Down
210 changes: 0 additions & 210 deletions coral-trino/src/main/java/com/linkedin/coral/trino/rel2trino/Calcite2TrinoUDFConverter.java
View file
Open in desktop

This file was deleted.

5 changes: 3 additions & 2 deletions ...ino/src/main/java/com/linkedin/coral/trino/rel2trino/DataTypeDerivedSqlCallConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2022-2023 LinkedIn Corporation. All rights reserved.
* Copyright 2022-2024 LinkedIn Corporation. All rights reserved.
* Licensed under the BSD-2 Clause license.
* See LICENSE in the project root for license information.
*/
Expand All @@ -16,6 +16,7 @@
import com.linkedin.coral.common.utils.TypeDerivationUtil;
import com.linkedin.coral.hive.hive2rel.HiveToRelConverter;
import com.linkedin.coral.hive.hive2rel.functions.VersionedSqlUserDefinedFunction;
import com.linkedin.coral.trino.rel2trino.transformers.CastOperatorTransformer;
import com.linkedin.coral.trino.rel2trino.transformers.ConcatOperatorTransformer;
import com.linkedin.coral.trino.rel2trino.transformers.FromUtcTimestampOperatorTransformer;
import com.linkedin.coral.trino.rel2trino.transformers.GenericProjectTransformer;
Expand Down Expand Up @@ -44,7 +45,7 @@ public DataTypeDerivedSqlCallConverter(HiveMetastoreClient mscClient, SqlNode to
operatorTransformerList = SqlCallTransformers.of(new FromUtcTimestampOperatorTransformer(typeDerivationUtil),
new GenericProjectTransformer(typeDerivationUtil), new NamedStructToCastTransformer(typeDerivationUtil),
new ConcatOperatorTransformer(typeDerivationUtil), new SubstrOperatorTransformer(typeDerivationUtil),
new UnionSqlCallTransformer(typeDerivationUtil));
new CastOperatorTransformer(typeDerivationUtil), new UnionSqlCallTransformer(typeDerivationUtil));
Copy link
Contributor

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 before UnionSqlCallTransformer

Copy link
Contributor Author

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 that CastOperatorTransformer would need to transform.

}

@Override
Expand Down
9 changes: 3 additions & 6 deletions coral-trino/src/main/java/com/linkedin/coral/trino/rel2trino/RelToTrinoConverter.java
View file
Open in desktop
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.
*/
Expand All @@ -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;
Expand All @@ -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.*;


Expand All @@ -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))
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you ignore the configs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

convertRel(relNode, configs) is a method from the now deleted Calcite2TrinoUDFConverter. I believe the logic of configs has been moved to CoralToTrinoSqlCallConverter

Copy link
Contributor

Choose a reason for hiding this comment

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

@aastha25 could you verify this change is safe?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be safe to remove.
Removing the Rel transformation layer, Calcite2TrinoUDFConverter, in Coral IR -> Trino SQL is a huge milestone. Thanks @KevinGe00 for getting us there :)


SqlNode sqlNodeWithRelDataTypeDerivedConversions =
sqlNode.accept(new DataTypeDerivedSqlCallConverter(_hiveMetastoreClient, sqlNode));
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/com/linkedin/coral/trino/rel2trino/transformers/AsOperatorTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2023 LinkedIn Corporation. All rights reserved.
* Copyright 2023-2024 LinkedIn Corporation. All rights reserved.
* Licensed under the BSD-2 Clause license.
* See LICENSE in the project root for license information.
*/
Expand Down
Loading
Loading