Skip to content

Conversation

@urosstan-db
Copy link
Contributor

@urosstan-db urosstan-db commented Nov 7, 2025

What changes were proposed in this pull request?

  • Support conversion of catalyst GetArrayItem expression to connector expression to allow data sources to implement pushdown of this expression

Why are the changes needed?

  • To allow data sources (built-in and third-party) to implement pushdown of get array item

Does this PR introduce any user-facing change?

No

How was this patch tested?

No testing needed, since we did not implement pushdowns yet

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the SQL label Nov 7, 2025
Copy link
Contributor

@milastdbx milastdbx left a comment

Choose a reason for hiding this comment

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

LGTM

case _: StringRPad => generateExpressionWithName("RPAD", expr, isPredicate)
case GetArrayItem(_, _, failOnError) if failOnError =>
// Pushdown only if ANSI is enabled (fail on error) to be compatible with remote systems.
generateExpressionWithName("GET_ARRAY_ITEM", expr, isPredicate)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can make new v2 expression as well to be able to pass failOnError argument, since get method in spark returns 0-indexed element of array, but it does not fail for index out of bounds, so it would be beneficial to pass failOnError to be able to support get method pushdown beside bracket access. WDYT @srielau @cloud-fan ?

"Failed to encode a value of the expressions: <expressions> to a row."
],
"sqlState" : "42846"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not find we have similar non legacy error. Legacy errors are usually too specific, e.g. unsupported function.


public Expression getChildArray() { return this.childArray; }
public Expression getOrdinal() { return this.ordinal; }
public boolean getFailOnError() { return this.failOnError; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the get prefix in other v2 expressions such as Cast. Can we be consistent here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, makes sense 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants