Skip to content

SAMZA-2455: Validate the argument types in SamzaSQL UDF on execution planning phase#1269

Merged
shanthoosh merged 8 commits into
apache:masterfrom
shanthoosh:udf-arg-validations
Feb 11, 2020
Merged

SAMZA-2455: Validate the argument types in SamzaSQL UDF on execution planning phase#1269
shanthoosh merged 8 commits into
apache:masterfrom
shanthoosh:udf-arg-validations

Conversation

@shanthoosh
Copy link
Copy Markdown
Contributor

@shanthoosh shanthoosh commented Feb 7, 2020

Issues: Incompatible types between UDF and schema is a irrecoverable error and it would kill the SamzaSQL job. Argument type validations in SamzaSQL UDF happens at runtime after the containers are launched. This causes operational inconvenience since these UDF type-errors causes a sql-job to fail at runtime.

Changes: This patch adds UDF validations to execution planning phase of SQL application. Operand checker in SamzaSQL is extended to house the udf argument type validation. This will ensure that the deployment of error-prone Samza SQL UDF will fail.

API changes: None

Upgrade/usage instructions: None

@shanthoosh shanthoosh changed the title Validate the argument types in SamzaSQL UDF on execution planning phase SAMZA-2455: Validate the argument types in SamzaSQL UDF on execution planning phase Feb 7, 2020
@shanthoosh shanthoosh requested a review from atoomula February 7, 2020 23:43
@shanthoosh
Copy link
Copy Markdown
Contributor Author

@weiqingy Can you please take a look at this PR?

Copy link
Copy Markdown
Contributor

@srinipunuru srinipunuru left a comment

Choose a reason for hiding this comment

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

LGTM

@srinipunuru
Copy link
Copy Markdown
Contributor

srinipunuru commented Feb 10, 2020

@santhoshvenkat1988 Couple of things that you could look at in future

  1. If adding validations with UDF polymorphism is hard, We could disable/remove it.
  2. May be an end to end planning test UDf type validation test would be great.
  3. Can you add description to the PR?

@shanthoosh
Copy link
Copy Markdown
Contributor Author

Thanks for the review @srinipunuru

  • I've updated the PR description.
  • I have added the end-to-end integration test for mismatched SamzaSQL UDF type.
  • Created a follow-up ticket to disallow the UDF polymorphism.

@shanthoosh shanthoosh merged commit ea49010 into apache:master Feb 11, 2020
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.

2 participants