pgjdbc/pgjdbc GitHub issues and pull requests (mirror)  
help / color / mirror / Atom feed
[pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
5+ messages / 4 participants
[nested] [flat]

* [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
@ 2024-03-21 15:22  "vishalvrv9 (@vishalvrv9)" <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: vishalvrv9 (@vishalvrv9) @ 2024-03-21 15:22 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

@vlsi @davecramer @sehrope 

This PR adheres to review comments provided in #3167 

- Validation of resultset params is done at the PgStatement constructor as PgPreparedStatement and PgCallableStatement inherit from there and the validation holds true at the base level
- localized messages are provided. Instead of SQL Exception, using PSQLException with invalid param errorcode
- tests now use assertThrows instead of fail

```./gradlew test```, ```./gradlew build```, and  ```./gradlew styleCheck``` are all passing

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
@ 2024-03-23 13:46  "sehrope (@sehrope)" <[email protected]>
  3 siblings, 0 replies; 5+ messages in thread

From: sehrope (@sehrope) @ 2024-03-23 13:46 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Very nice.

Comparing this branch against the state of the repo just before the prior attempt at this (i.e. pretend the previous commit didn't happen) shows a nice clean diff and makes it easy to see both the change and its correctness:

```diff
$ git diff 24f2c7eea4764cd4e09c2c71bc8c38ee5d4178e5 pgjdbc/src/main/java/
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
index 315781d4..ab574c18 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
@@ -1408,16 +1408,14 @@ public class PgConnection implements BaseConnection {
   public PreparedStatement prepareStatement(String sql, int resultSetType, int resultSetConcurrency,
       int resultSetHoldability) throws SQLException {
     checkClosed();
-    return new PgPreparedStatement(this, sql, resultSetType, resultSetConcurrency,
-        resultSetHoldability);
+    return new PgPreparedStatement(this, sql, resultSetType, resultSetConcurrency, resultSetHoldability);
   }
 
   @Override
   public CallableStatement prepareCall(String sql, int resultSetType, int resultSetConcurrency,
       int resultSetHoldability) throws SQLException {
     checkClosed();
-    return new PgCallableStatement(this, sql, resultSetType, resultSetConcurrency,
-        resultSetHoldability);
+    return new PgCallableStatement(this, sql, resultSetType, resultSetConcurrency, resultSetHoldability);
   }
 
   @Override
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
index 944d56eb..98dd512e 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
@@ -161,11 +161,26 @@ public class PgStatement implements Statement, BaseStatement {
       throws SQLException {
     this.connection = c;
     forceBinaryTransfers |= c.getForceBinary();
+    // validation check for allowed values of resultset type
+    if (rsType != ResultSet.TYPE_FORWARD_ONLY && rsType != ResultSet.TYPE_SCROLL_INSENSITIVE && rsType != ResultSet.TYPE_SCROLL_SENSITIVE) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet type"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     resultsettype = rsType;
+    // validation check for allowed values of resultset concurrency
+    if (rsConcurrency != ResultSet.CONCUR_READ_ONLY && rsConcurrency != ResultSet.CONCUR_UPDATABLE) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet concurrency"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     concurrency = rsConcurrency;
     setFetchSize(c.getDefaultFetchSize());
     setPrepareThreshold(c.getPrepareThreshold());
     setAdaptiveFetch(c.getAdaptiveFetch());
+    // validation check for allowed values of resultset holdability
+    if (rsHoldability != ResultSet.HOLD_CURSORS_OVER_COMMIT && rsHoldability != ResultSet.CLOSE_CURSORS_AT_COMMIT) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet holdability"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     this.rsHoldability = rsHoldability;
   } 
```

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
@ 2025-05-30 19:43  "vlsi (@vlsi)" <[email protected]>
  3 siblings, 0 replies; 5+ messages in thread

From: vlsi (@vlsi) @ 2025-05-30 19:43 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java:166)

Error message should better include the problematic value in question. The current error makes it hard to understand the reason for the failure.

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
@ 2025-05-30 20:02  "vishalvrv9 (@vishalvrv9)" <[email protected]>
  3 siblings, 0 replies; 5+ messages in thread

From: vishalvrv9 (@vishalvrv9) @ 2025-05-30 20:02 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java:166)

I can take this up. 

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
@ 2025-05-30 20:04  "davecramer (@davecramer)" <[email protected]>
  3 siblings, 0 replies; 5+ messages in thread

From: davecramer (@davecramer) @ 2025-05-30 20:04 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java:166)

Please create an issue for it and assign yourself to it. There are other places as well

^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2025-05-30 20:04 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-21 15:22 [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro… "vishalvrv9 (@vishalvrv9)" <[email protected]>
2024-03-23 13:46 ` "sehrope (@sehrope)" <[email protected]>
2025-05-30 19:43 ` "vlsi (@vlsi)" <[email protected]>
2025-05-30 20:02 ` "vishalvrv9 (@vishalvrv9)" <[email protected]>
2025-05-30 20:04 ` "davecramer (@davecramer)" <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox