Skip to content

Conversation

@zhztheplayer
Copy link
Member

@zhztheplayer zhztheplayer commented Nov 1, 2025

What changes were proposed in this pull request?

Add the following code in HashedRelationSuite, to cover the API HashedRelation#close in the test suite.

  protected override def afterEach(): Unit = {
    super.afterEach()
    assert(umm.executionMemoryUsed === 0)
  }

Why are the changes needed?

Doing this will:

  1. Ensure HashedRelation#close is called in test code, to lower memory footprint and avoid memory leak when executing tests.
  2. Ensure implementations of HashedRelation#close free the allocated memory blocks correctly.

It's an individual effort to improve the test quality, but also a prerequisite task for #52817.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

It's a test PR.

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

No.

@github-actions github-actions bot added the SQL label Nov 1, 2025
@zhztheplayer
Copy link
Member Author

@HyukjinKwon @yaooqinn @wangyum @cloud-fan Could you please review? Thanks!

@cloud-fan
Copy link
Contributor

thanks, merging to master/4.1 (for test coverage)!

@cloud-fan cloud-fan closed this in a5e866f Nov 3, 2025
cloud-fan pushed a commit that referenced this pull request Nov 3, 2025
…nSuite

### What changes were proposed in this pull request?

Add the following code in `HashedRelationSuite`, to cover the API `HashedRelation#close` in the test suite.

```scala
  protected override def afterEach(): Unit = {
    super.afterEach()
    assert(umm.executionMemoryUsed === 0)
  }
```

### Why are the changes needed?

Doing this will:

1. Ensure `HashedRelation#close` is called in test code, to lower memory footprint and avoid memory leak when executing tests.
2. Ensure implementations of `HashedRelation#close` free the allocated memory blocks correctly.

It's an individual effort to improve the test quality, but also a prerequisite task for #52817.

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

No.

### How was this patch tested?

It's a test PR.

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

No.

Closes #52830 from zhztheplayer/wip-54132.

Authored-by: Hongze Zhang <hongze.zzz123@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit a5e866f)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@zhztheplayer
Copy link
Member Author

Thanks, @cloud-fan!

@yaooqinn
Copy link
Member

yaooqinn commented Nov 4, 2025

Late LGTM

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