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

Make Coral IR friendly to missing table alias prefix #449

Merged
merged 8 commits into from
Sep 8, 2023

Conversation

yiqiangin
Copy link
Contributor

@yiqiangin yiqiangin commented Sep 7, 2023

What changes are proposed in this pull request, and why are they necessary?

As Coral IR depends on the code of Calcite to generate RelDataType for each column, RelDataType of a column of struct type is always generated with StructKind.FULLY_QUALIFIED as shown in RelDataTypeFactoryImpl. Therefore SqlValidator from Calcite always requires a fully qualified name of a field from a column of struct type like tbl.structCol.field. If the query defines a field without table alias prefix like structCol.field, SqlValidator takes structCol as the table name and throw an exeption with the message of table structCol not found.
In order to make Coral IR work without table alias prefix for a field from a column of struct type, the following changes are made:

  • Adding a new class of CoraJavaTypeFactoryImpl which extends the class of JavaTypeFactoryImpl (which is a subclass of RelDataTypeFactoryImpl) and it overrides the function of createStructField with StructKind.PEEK_FIELDS_NO_EXPAND. Noted that the reason why StructKind.PEEK_FIELDS_NO_EXPANDis used not ``StructKind.PEEK_FIELDSorStructKind.PEEK_FIELDS_DEFAULT` is the other two options would make the translation of `select *` not work as expected.
  • RelDataTypeFactory associated with RelOptCluster is created in the code path of Frameworks.withPrepare(FrameworkConfig config,BasePrepareAction<R> action) which are all the code from Calcite which is called in RelBuilder.create(), however we cannot change the code in Calcite to create an object of CoralJavaTypeFactoryImpl in this path. Therefore a new class of HiveRelBuilder is created to override the function of RelBuilder.create() where an object of CoralJavaTypeFactoryImpl and associated RelOptCluster is created to replace the object of JavaTypeFactoryImpl and associated RelOptCluster which are created in the code path of Calcite to create an object of RelBuilder. It is used by HiveToRelConverter.

How was this patch tested?

  • Adding some unit tests as HiveToTrinoConverterTest.testSqlSelectAliasAppenderTransformerWithoutTableAliasPrefix, CoralSparkTest.testStructColProjectionWithoutTableAliasPrefix and CoralSparkTest.testStructColProjectionWithTableAliasPrefix
  • Fixing some existing unit test cases
  • ./gradlew clean build
  • manual test on a view where the field does not have table alias prefix in its View Expanded Text

Copy link
Contributor

@aastha25 aastha25 left a comment

Choose a reason for hiding this comment

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

thanks for the PR @yiqiangin

@yiqiangin yiqiangin merged commit c2f558d into linkedin:master Sep 8, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants