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

Add EXTRACT_COLLECTION UDF to registry #533

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Add EXTRACT_COLLECTION UDF to registry #533

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27ce9d9
Add EXTRACT_COLLECTION UDF to registry
Sep 17, 2024
40d7ad1
Change return type to address comments
Sep 19, 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: 2 additions & 0 deletions .../src/main/java/com/linkedin/coral/hive/hive2rel/functions/StaticHiveFunctionRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,8 @@ public boolean isOptional(int i) {
SqlTypeFamily.BOOLEAN, SqlTypeFamily.ANY, SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING));
createAddUserDefinedFunction("com.linkedin.groot.runtime.udf.spark.GetMappedValueUDF", FunctionReturnTypes.STRING,
family(SqlTypeFamily.STRING, SqlTypeFamily.STRING));
createAddUserDefinedFunction("com.linkedin.groot.runtime.udf.spark.ExtractCollectionUDF",
FunctionReturnTypes.arrayOfType(SqlTypeName.ANY, true), family(SqlTypeFamily.ANY, SqlTypeFamily.STRING));
Copy link
Collaborator

Choose a reason for hiding this comment

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

return type shouldn't contain SqlTypeName.ANY, is this the same type as the first parameter's type?

Copy link
Member

Choose a reason for hiding this comment

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

@ljfgem I discussed with Qiang offline, this UDF's return type is pretty complicated, it depends on its input arg type via some complex logic, and I'm not sure if it's even possible to express all that type in calcite. So I think using a broader arrayOfType of ANY type to express it should be fine here? I understand it will be a wider type than what this function strictly is, but it shouldn't cause any problems as long as the user of this udf knows what it's doing and uses it correctly with expected operand types.

Copy link
Collaborator

@ljfgem ljfgem Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IIRC, we encountered a similar issue while generating Avro schema for the UDF field because RelDataTypeToAvroType#basicSqlTypeToAvroType doesn't support ANY type and would throw exception.

If coral-schema fails, the UDF couldn't be registered properly and Spark query would fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The inputs are 1. an array field in dataframe which array contains struct and potentially nested arrays. 2. a string path reference from (1) to a primitive leaf field (int, long, string etc).
The return type will be an array of the type of leaf field, which type is only available at runtime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ljfgem If ANY is not possible, is there any way to set it to a union type? Something like Array<Int, Long and String>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to FunctionReturnTypes.arrayOfType(SqlTypeName.VARCHAR, true)
Please review again. Thanks

Copy link

Choose a reason for hiding this comment

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

Entry looks fine to me


// UDTFs
addFunctionEntry("explode", new CoralSqlUnnestOperator(false));
Expand Down
Loading