Skip to content

GROOVY-11541: Sql wildcard params - #2139

Merged
daniellansun merged 1 commit into
apache:masterfrom
seregamorph:GROOVY-11541-sql-wildcard-params
Jan 4, 2025
Merged

GROOVY-11541: Sql wildcard params#2139
daniellansun merged 1 commit into
apache:masterfrom
seregamorph:GROOVY-11541-sql-wildcard-params

Conversation

@seregamorph

@seregamorphseregamorph commented Jan 3, 2025

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/GROOVY-11541
groovy.sql.Sql declares several methods with SQL statement and parameters like this

publicbooleanexecute(Stringsql, List<Object> params) throwsSLQException {

This declaration has one major drawback: it's not possible to pass as an argument the List which is not exactly List<Object>, e.g. List<Integer>.

This List is only iterated, hence using wildcard here is safe. Also changing the type to wildcard as receiving argument should be binary compatible.

Suggested method signature (example; address all such methods of Sql class):

publicbooleanexecute(Stringsql, List<?> params) throwsSLQException {

Additional notes:

  • there are few methods that return List<Object>, e.g. SqlWithParams.getParams() - in this case the Object generic should be preserved.
  • For reference: pretty similar problem solved for cassandra-java-driver apache/cassandra-java-driver@be07a77

@codecov-commenter

codecov-commenter commented Jan 3, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.8300%. Comparing base (3d97ab7) to head (fcfe772).
⚠️ Report is 1010 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@ Coverage Diff @@## master #2139 +/- ##
=============================================
Coverage 68.8300% 68.8300% + Complexity 29427 29425 -2 
=============================================
Files 1420 1420 Lines 113141 113141 Branches 19547 19547 =============================================
Hits 77875 77875 - Misses 28724 28725 +1 + Partials 6542 6541 -1 
Files with missing linesCoverage Δ
...jects/groovy-sql/src/main/java/groovy/sql/Sql.java64.6637% <100.0000%> (ø)
...vy-sql/src/main/java/groovy/sql/SqlWithParams.java100.0000% <100.0000%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seregamorph

Copy link
Copy Markdown
ContributorAuthor

The build is broken, but doesn't seem related as master branch is also broken: https://github.com/apache/groovy/actions/runs/12586627965/job/35088979045

@daniellansun

Copy link
Copy Markdown
Contributor

LGTM.

No worries. JDK 13 and 18 builds fail due to timeout unaccountably someday, it is tricky for us too.

@daniellansun
daniellansun merged commit 015fda4 into apache:masterJan 4, 2025
@daniellansun

Copy link
Copy Markdown
Contributor

Merged. Thanks!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@seregamorph@codecov-commenter@daniellansun