Skip to content

Conversation

@dtenedor
Copy link
Contributor

What changes were proposed in this pull request?

This PR adds a small refactor of the SQL pipe syntax analysis code for sharing and reuse.

Why are the changes needed?

The new analyzer can refer to the refactored code without duplicating it.

Does this PR introduce any user-facing change?

No

How was this patch tested?

N/A, this is a small refactoring only.

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

No

@github-actions github-actions bot added the SQL label Dec 10, 2025
def validateAndStripPipeExpression(
pipeExpression: PipeExpression,
resolvedChild: Expression): Expression = {
val firstAggregateFunction: Option[AggregateFunction] = findFirstAggregate(resolvedChild)
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, we should extract this outside this method and then pass it as an arg, because it is recursive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review, I looked at this, it turns out we are always going to want to use the same expression-finding logic, so should be OK to leave this hard-coded in here.

@dtenedor
Copy link
Contributor Author

I just looked at the test failure, it is unrelated.

@dtenedor
Copy link
Contributor Author

Thanks all for review, merging to master.

@dtenedor dtenedor closed this in d57136c Dec 11, 2025
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.

4 participants