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 'valueIn' MV transform function work with the multi-stage query engine #13443

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

yashmayya
Copy link
Collaborator

  • The valueIn / VALUE_IN transform function didn't work with the multi-stage query engine and resulted in errors like No match found for function signature valueIn(<VARCHAR ARRAY>, <CHARACTER>, <CHARACTER>).
  • It didn't have operand type checkers and return type inferences defined in TransformFunctionType which are used while registering the Pinot transform functions in Calcite's operator table.
  • The VALUE_IN function takes a multi-value column as its first input, followed by any number of constant value arguments. Hence, we're using Calcite's variadic operand type checker in this patch.
  • While there exist more powerful type checkers that could ensure things like the first argument is an array and the second argument is the type of the array's elements and so on, these don't seem to be combinable with the variadic type checker. However, this shouldn't be an issue because the transform function itself ensures that the first argument is a multi-value column and even in the v1 query engine, the subsequent arguments are allowed to be of any type.
  • Alternatively, we could use the family operand type checker with an arbitrarily large number of subsequent arguments (after the first one being a SqlTypeFamily.ARRAY) with all but the first two being optional. This would however introduce an artificial limitation on the number of operands that the VALUE_IN function can take (also it would look fairly ugly - both in code, as well as Calcite's generated function signature).

@yashmayya yashmayya requested review from gortiz and Jackie-Jiang June 20, 2024 05:31
@yashmayya yashmayya added bugfix multi-stage Related to the multi-stage query engine labels Jun 20, 2024
@yashmayya yashmayya force-pushed the value-in-v2-engine branch from 63011ec to 4243b4f Compare June 20, 2024 07:01
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM!

@Jackie-Jiang Jackie-Jiang merged commit 3224b36 into apache:master Jun 20, 2024
20 checks passed
suyashpatel98 pushed a commit to suyashpatel98/pinot that referenced this pull request Jul 6, 2024
@npawar npawar added the v1v2 label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix multi-stage Related to the multi-stage query engine v1v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants