Skip to content

Comments

[SPARK-45109][SQL][CONNECT][FOLLOWUP] Fix log function in Connect#42869

Closed
peter-toth wants to merge 1 commit intoapache:masterfrom
peter-toth:SPARK-45109-fix-log
Closed

[SPARK-45109][SQL][CONNECT][FOLLOWUP] Fix log function in Connect#42869
peter-toth wants to merge 1 commit intoapache:masterfrom
peter-toth:SPARK-45109-fix-log

Conversation

@peter-toth
Copy link
Contributor

What changes were proposed in this pull request?

This is a follow-up PR to #42863, the 1 argument log function should also point to ln.

Why are the changes needed?

Bugfix.

Does this PR introduce any user-facing change?

No, these Spark Connect functions haven't been released.

How was this patch tested?

Exsiting UTs.

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

No.

@peter-toth
Copy link
Contributor Author

@HyukjinKwon
Copy link
Member

For some reasons, it can't find the build. The build is here: https://github.com/peter-toth/spark/actions/runs/6143939836/job/16668358616

@peter-toth
Copy link
Contributor Author

peter-toth commented Sep 11, 2023

For some reasons, it can't find the build. The build is here: https://github.com/peter-toth/spark/actions/runs/6143939836/job/16668358616

Yeah I noticed that and force pushed a new commit so test are running now.
But I've run into this issue a few times, it happened here too: #42863 (comment)

* @group math_funcs
* @since 3.4.0
*/
def log(e: Column): Column = Column.fn("log", e)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to directly backport this fix to 3.4, I think Column.fn("ln", e) maybe better, otherwise, we may need separate PR for 3.4 since def ln was added in 3.5.

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 will open a 3.4 version later.

peter-toth added a commit that referenced this pull request Sep 11, 2023
### What changes were proposed in this pull request?
This is a follow-up PR to #42863, the 1 argument `log` function should also point to `ln`.

### Why are the changes needed?
Bugfix.

### Does this PR introduce _any_ user-facing change?
No, these Spark Connect functions haven't been released.

### How was this patch tested?
Exsiting UTs.

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

Closes #42869 from peter-toth/SPARK-45109-fix-log.

Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Peter Toth <peter.toth@gmail.com>
(cherry picked from commit 6c3d9f5)
Signed-off-by: Peter Toth <peter.toth@gmail.com>
@peter-toth
Copy link
Contributor Author

Thanks for the review! Merged to master/3.5.

@peter-toth
Copy link
Contributor Author

3.4 PR: #42872

dongjoon-hyun pushed a commit that referenced this pull request Sep 12, 2023
### What changes were proposed in this pull request?
This is a backport PR of #42869 as the 1 argument `log` function should point to `ln`. (Please note that the original #42863 doesn't need to be backported as `aes_descrypt` and `ln` is not implemented in Connect in Spark 3.4.)

### Why are the changes needed?
Bugfix.

### Does this PR introduce _any_ user-facing change?
No, these Spark Connect functions haven't been released.

### How was this patch tested?
Exsiting UTs.

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

Closes #42872 from peter-toth/SPARK-45109-fix-log-3.4.

Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
viirya pushed a commit to viirya/spark-1 that referenced this pull request Oct 19, 2023
### What changes were proposed in this pull request?
This is a backport PR of apache#42869 as the 1 argument `log` function should point to `ln`. (Please note that the original apache#42863 doesn't need to be backported as `aes_descrypt` and `ln` is not implemented in Connect in Spark 3.4.)

### Why are the changes needed?
Bugfix.

### Does this PR introduce _any_ user-facing change?
No, these Spark Connect functions haven't been released.

### How was this patch tested?
Exsiting UTs.

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

Closes apache#42872 from peter-toth/SPARK-45109-fix-log-3.4.

Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
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.

5 participants