Skip to content

Conversation

@vladimirg-db
Copy link
Contributor

What changes were proposed in this pull request?

Factor out function resolution code to the FunctionResolution object.

Why are the changes needed?

Some Analyzer code will be used in both fixed-point and single-pass Analyzers. Also, Analyzer.scala is 4K+ lines long, so it makes sense to gradually split it.

Context: https://issues.apache.org/jira/browse/SPARK-49834

Does this PR introduce any user-facing change?

No. It's a pure refactoring.

How was this patch tested?

Existing tests.

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

No.

@github-actions github-actions bot added the SQL label Nov 15, 2024
extends DataTypeErrorsBase with LookupCatalog {
private val v1SessionCatalog = catalogManager.v1SessionCatalog

private val trimWarningEnabled = new AtomicBoolean(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this atomic boolean was in object FunctionResolution inside class Analyzer, we have to move it to class FunctionResolution, and not to global object FunctionResolution. @MaxGekk please correct me if I'm wrong.

Copy link
Member

Choose a reason for hiding this comment

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

sgtm

extends DataTypeErrorsBase with LookupCatalog {
private val v1SessionCatalog = catalogManager.v1SessionCatalog

private val trimWarningEnabled = new AtomicBoolean(true)
Copy link
Member

Choose a reason for hiding this comment

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

sgtm

@MaxGekk
Copy link
Member

MaxGekk commented Nov 15, 2024

+1, LGTM. Merging to master.
Thank you, @vladimirg-db.

@MaxGekk MaxGekk closed this in 77e006f Nov 15, 2024
@iwanttobepowerful
Copy link

Hi, @vladimirg-db Do you have the JIRAs related to the Single-pass Analyzer work aggregated into a certain epic? This would facilitate tracking.

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