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

[SPARK-49909][SQL] Fix the pretty name of some expressions #48385

Closed
wants to merge 2 commits into from

Conversation

panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Oct 8, 2024

What changes were proposed in this pull request?

The pr aims to fix the pretty name of some expressions, includes: random, to_varchar, current_database, curdate, dateadd and array_agg.

Why are the changes needed?

The actual function name used does not match the displayed name, as shown below:

  • Before:
image
  • After:
image

Does this PR introduce any user-facing change?

Yes, Make the header of the data seen by the end-user from Spark SQL consistent with the actual function name used.

How was this patch tested?

  • Pass GA.
  • Update existed UT.

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

No.

@github-actions github-actions bot added the SQL label Oct 8, 2024
@panbingkun panbingkun marked this pull request as ready for review October 8, 2024 14:17
@MaxGekk
Copy link
Member

MaxGekk commented Oct 9, 2024

+1, LGTM. Merging to master.
Thank you, @panbingkun and @HyukjinKwon for review.

@MaxGekk MaxGekk closed this in 52538f0 Oct 9, 2024
@MaxGekk
Copy link
Member

MaxGekk commented Oct 9, 2024

@panbingkun I think we should backport the fix. Could you open separate PRs, please, because your changes cause conflicts in branch-3.5 (and maybe in branch-3.4).

@panbingkun
Copy link
Contributor Author

@panbingkun I think we should backport the fix. Could you open separate PRs, please, because your changes cause conflicts in branch-3.5 (and maybe in branch-3.4).

Sure, allow me to complete it.

MaxGekk pushed a commit that referenced this pull request Oct 9, 2024
### What changes were proposed in this pull request?
The pr aims to fix the `pretty name` of some `expressions`, includes: `random`, `to_varchar`, `current_database`, `curdate`, `dateadd` and `array_agg`.
(PS: The pr is backport branch-3.5, master pr is: #48385)

### Why are the changes needed?
The actual function name used does not match the displayed name, as shown below:
- Before:
<img width="573" alt="image" src="https://github.com/user-attachments/assets/f5785c80-f6cb-494f-a15e-9258eca688a7">

- After:
<img width="570" alt="image" src="https://github.com/user-attachments/assets/792a7092-ccbf-49f4-a616-19110e5c2361">

### Does this PR introduce _any_ user-facing change?
Yes, Make the header of the data seen by the end-user from `Spark SQL` consistent with the `actual function name` used.

### How was this patch tested?
- Pass GA.
- Update existed UT.

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

Closes #48393 from panbingkun/branch-3.5_SPARK-49909.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
MaxGekk pushed a commit that referenced this pull request Oct 10, 2024
### What changes were proposed in this pull request?
The pr aims to fix the `pretty name` of some `expressions`, includes: `random`, `to_varchar`, `current_database`, `curdate`, `dateadd` and `array_agg`.
(PS: The pr is backport branch-3.4, master pr is: #48385)

### Why are the changes needed?
The actual function name used does not match the displayed name, as shown below:
- Before:
<img width="573" alt="image" src="https://github.com/user-attachments/assets/f5785c80-f6cb-494f-a15e-9258eca688a7">

- After:
<img width="570" alt="image" src="https://github.com/user-attachments/assets/792a7092-ccbf-49f4-a616-19110e5c2361">

### Does this PR introduce _any_ user-facing change?
Yes, Make the header of the data seen by the end-user from `Spark SQL` consistent with the `actual function name` used.

### How was this patch tested?
- Pass GA.
- Update existed UT.

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

Closes #48396 from panbingkun/branch-3.4_SPARK-49909.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

The benefit of this fix is minimal, but it will disrupt the queries of many end users because they rely on the auto-generated alias.

@panbingkun
Copy link
Contributor Author

panbingkun commented Oct 18, 2024

  • I overlooked another scenario in the PR description, when an error occurs, the function name prompted may not match the actual function name, eg:
image
spark-sql (default)> select random("1");
[DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE] Cannot resolve "rand(1)" due to data type mismatch: The first parameter requires the ("INT" or "BIGINT") type, however "1" has the type "STRING". SQLSTATE: 42K09; line 1 pos 7;
'Project [unresolvedalias(random(1))]
+- OneRowRelation

@panbingkun
Copy link
Contributor Author

revert pr: #48530

@panbingkun
Copy link
Contributor Author

After the master branch merges the revert PR, I will submit PRs to branch-3.5 and branch-3.4 respectively to revert these changes.

@panbingkun
Copy link
Contributor Author

branch-3.5 revert: #48531
branch-3.4 revert: #48532

MaxGekk pushed a commit that referenced this pull request Oct 18, 2024
### What changes were proposed in this pull request?
The pr aims to revert #48385.
This reverts commit 52538f0.

### Why are the changes needed?
When upgrading spark from `an old version` to `the latest version`, some end-users may rely on the `original schema` (`although it may not be correct`), which can make the `upgrade` very difficult. so, let's first restore it to its original state.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA

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

Closes #48530 from panbingkun/SPARK-49909_revert.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
himadripal pushed a commit to himadripal/spark that referenced this pull request Oct 19, 2024
### What changes were proposed in this pull request?
The pr aims to fix the `pretty name` of some `expressions`, includes: `random`, `to_varchar`, `current_database`, `curdate`, `dateadd` and `array_agg`.

### Why are the changes needed?
The actual function name used does not match the displayed name, as shown below:
- Before:
<img width="573" alt="image" src="https://github.com/user-attachments/assets/f5785c80-f6cb-494f-a15e-9258eca688a7">

- After:
<img width="570" alt="image" src="https://github.com/user-attachments/assets/792a7092-ccbf-49f4-a616-19110e5c2361">

### Does this PR introduce _any_ user-facing change?
Yes, Make the header of the data seen by the end-user from `Spark SQL` consistent with the `actual function name` used.

### How was this patch tested?
- Pass GA.
- Update existed UT.

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

Closes apache#48385 from panbingkun/SPARK-49909.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
himadripal pushed a commit to himadripal/spark that referenced this pull request Oct 19, 2024
### What changes were proposed in this pull request?
The pr aims to revert apache#48385.
This reverts commit 52538f0.

### Why are the changes needed?
When upgrading spark from `an old version` to `the latest version`, some end-users may rely on the `original schema` (`although it may not be correct`), which can make the `upgrade` very difficult. so, let's first restore it to its original state.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA

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

Closes apache#48530 from panbingkun/SPARK-49909_revert.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants