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

@seregamorph seregamorph 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

public boolean execute(String sql, List<Object> params) throws SLQException {

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):

public boolean execute(String sql, List<?> params) throws SLQException {

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 lines Coverage Δ
...jects/groovy-sql/src/main/java/groovy/sql/Sql.java 64.6637% <100.0000%> (ø)
...vy-sql/src/main/java/groovy/sql/SqlWithParams.java 100.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
Contributor Author

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:master Jan 4, 2025
@daniellansun

Copy link
Copy Markdown
Contributor

Merged. Thanks!

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

Labels

None yet

3 participants