Skip to content

Add test coverage for SqlFunctionProperties equals#25002

Merged
duxiao1212 merged 1 commit intoprestodb:masterfrom
duxiao1212:dev-add-unit-tests-for-sqlfunctionproperties
Apr 30, 2025
Merged

Add test coverage for SqlFunctionProperties equals#25002
duxiao1212 merged 1 commit intoprestodb:masterfrom
duxiao1212:dev-add-unit-tests-for-sqlfunctionproperties

Conversation

@duxiao1212
Copy link
Contributor

@duxiao1212 duxiao1212 commented Apr 29, 2025

Description

Previously, in the PR #24614, we accidentally compared different fields in the SqlFunctionProperties, which caused an OOM error in the JVM metaspace and eventually led to a SEV. To prevent this issue from occurring again, we need to ensure that the SqlFunctionProperties equality function is implemented correctly. Additionally, we should create a unit test to reproduce the issue and prevent it from happening in the future.

Motivation and Context

Prevent incorrect implementation of the equals method in SqlFunctionProperties

Impact

low impact

Test Plan

N/A

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== NO RELEASE NOTE ==

@duxiao1212 duxiao1212 requested review from a team and elharo as code owners April 29, 2025 02:18
@duxiao1212 duxiao1212 requested a review from jaystarshot April 29, 2025 02:18
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Apr 29, 2025
@duxiao1212 duxiao1212 force-pushed the dev-add-unit-tests-for-sqlfunctionproperties branch 2 times, most recently from 879fd21 to d3e755a Compare April 29, 2025 16:31
@duxiao1212 duxiao1212 requested a review from rschlussel April 29, 2025 19:49
rschlussel
rschlussel previously approved these changes Apr 29, 2025
@duxiao1212 duxiao1212 force-pushed the dev-add-unit-tests-for-sqlfunctionproperties branch from d3e755a to c33b931 Compare April 29, 2025 21:31
@duxiao1212 duxiao1212 force-pushed the dev-add-unit-tests-for-sqlfunctionproperties branch from c33b931 to 85f2c3e Compare April 30, 2025 15:25
@duxiao1212 duxiao1212 merged commit a6ecdb5 into prestodb:master Apr 30, 2025
97 checks passed
@ZacBlanco ZacBlanco mentioned this pull request May 29, 2025
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:Meta PR from Meta

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants