pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
20+ messages / 3 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-05 19:40 "tirthgajera (@tirthgajera)" <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-05 19:40 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
### All Submissions:
* [x] Have you followed the guidelines in our [Contributing](https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md) document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](../../pulls) for the same update/change?
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### New Feature Submissions:
1. [x] Does your submission pass tests?
2. [x] Does `./gradlew styleCheck` pass ?
3. [x] Have you added your new test classes to an existing test suite in alphabetical order?
### Changes to Existing Features:
* [x] Does this break existing behaviour? If so please explain.
* [ ] Have you added an explanation of what your changes do and why you'd like us to include them?
* [x] Have you written new tests for your core changes, as applicable?
* [x] Have you successfully run tests with your changes locally?
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-05 21:39 "davecramer (@davecramer)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: davecramer (@davecramer) @ 2025-04-05 21:39 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Please add a test for all of the cases
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-06 12:30 "tirthgajera (@tirthgajera)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-06 12:30 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> Please add a test for all of the cases
Done!! testcase added.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-06 20:04 "davecramer (@davecramer)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: davecramer (@davecramer) @ 2025-04-06 20:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
CHAR_OCTET_LENGTH returns an int, not a string
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-09 18:00 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-09 18:00 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java)
Could you add `NCHAR`, `NVARCHAR`, and `LONGNVARCHAR` as well?
It might be we should add `CLOB` here as well, however, I am not sure.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-09 18:02 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-09 18:02 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java)
Great that you add assert messages, however, `expected null` does not add much since `assertNull` will add it anyways.
```suggestion
assertNull(rs.getString("CHAR_OCTET_LENGTH"), "CHAR_OCTET_LENGTH for integer[]");
```
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-09 18:02 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-09 18:02 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java)
Same thing, `expected 100` will appear in the message automatically.
```suggestion
assertEquals("100", rs.getString("CHAR_OCTET_LENGTH"), "CHAR_OCTET_LENGTH for varchar(100)");
```
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-09 18:04 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-09 18:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java)
The test method name looks outdated: it verifies CHAR_OCTET_LENGTH for several types, not just `char`.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-09 18:05 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-09 18:05 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java)
Have you considered factoring out it to a helper method?
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-10 09:38 "tirthgajera (@tirthgajera)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-10 09:38 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java)
need to add clob or?
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-10 09:55 "davecramer (@davecramer)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: davecramer (@davecramer) @ 2025-04-10 09:55 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java)
why wouldn't we ?
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-10 10:11 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-10 10:11 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java)
The spec is not very clear. I would check the way MSSQL, and MySQL handles this.
However, CLOB "looks like text" and BLOB "looks like a binary", so it might be reasonable to add them both.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-10 20:21 "davecramer (@davecramer)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: davecramer (@davecramer) @ 2025-04-10 20:21 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java)
> The spec is not very clear. I would check the way MSSQL, and MySQL handles this. However, CLOB "looks like text" and BLOB "looks like a binary", so it might be reasonable to add them both.
I think it's fine to add them both as noted above
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-12 13:13 "tirthgajera (@tirthgajera)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-12 13:13 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java)
Hi @davecramer , @vlsi,
Changes pushed!!
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-13 08:19 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-13 08:19 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1971)
Please move `create table` to `@BeforeAll`, and `drop table` to `@AfterAll`. It would reduce the number of drop/create during the test.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-13 08:21 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-13 08:21 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1991)
A tiny note: lambdas could help with lazy execution, so the test does not spend time on building the failure message in case of the passing tests.
```suggestion
assertTrue(rs.next(), () -> "dbmd.getColumns returned no rows for column " + columnName);
```
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-13 08:24 "vlsi (@vlsi)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: vlsi (@vlsi) @ 2025-04-13 08:24 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1999)
The failure would maks the root cause. In most of the cases, I want to know the exact expected/actual value rather than just the fact "it should not be null".
Have you considered something like the following?
```java
Integer actual = rs.wasNull() ? null : rs.getInt("CHAR_OCTET_LENGTH");
assertEquals(expected, actual, ...);
```
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-18 21:34 "tirthgajera (@tirthgajera)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-18 21:34 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1971)
In test we already have BeforeEach and AfterEach. Is it okay if I add create and drop table there.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-18 21:37 "davecramer (@davecramer)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: davecramer (@davecramer) @ 2025-04-18 21:37 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1971)
Why not put it in @BeforeAll and @AfterAll ?
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types
@ 2025-04-18 21:53 "tirthgajera (@tirthgajera)" <[email protected]>
18 siblings, 0 replies; 20+ messages in thread
From: tirthgajera (@tirthgajera) @ 2025-04-18 21:53 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/DatabaseMetaDataTest.java:1971)
Yes, we can.
I'm introducing new beforeAll and afterAll for this test and adding it to it.
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2025-04-18 21:53 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-04-05 19:40 [pgjdbc/pgjdbc] PR #3589: Metadata CHAR_OCTET_LENGTH is a positive number for non-character column types "tirthgajera (@tirthgajera)" <[email protected]>
2025-04-05 21:39 ` "davecramer (@davecramer)" <[email protected]>
2025-04-06 12:30 ` "tirthgajera (@tirthgajera)" <[email protected]>
2025-04-06 20:04 ` "davecramer (@davecramer)" <[email protected]>
2025-04-09 18:00 ` "vlsi (@vlsi)" <[email protected]>
2025-04-09 18:02 ` "vlsi (@vlsi)" <[email protected]>
2025-04-09 18:02 ` "vlsi (@vlsi)" <[email protected]>
2025-04-09 18:04 ` "vlsi (@vlsi)" <[email protected]>
2025-04-09 18:05 ` "vlsi (@vlsi)" <[email protected]>
2025-04-10 09:38 ` "tirthgajera (@tirthgajera)" <[email protected]>
2025-04-10 09:55 ` "davecramer (@davecramer)" <[email protected]>
2025-04-10 10:11 ` "vlsi (@vlsi)" <[email protected]>
2025-04-10 20:21 ` "davecramer (@davecramer)" <[email protected]>
2025-04-12 13:13 ` "tirthgajera (@tirthgajera)" <[email protected]>
2025-04-13 08:19 ` "vlsi (@vlsi)" <[email protected]>
2025-04-13 08:21 ` "vlsi (@vlsi)" <[email protected]>
2025-04-13 08:24 ` "vlsi (@vlsi)" <[email protected]>
2025-04-18 21:34 ` "tirthgajera (@tirthgajera)" <[email protected]>
2025-04-18 21:37 ` "davecramer (@davecramer)" <[email protected]>
2025-04-18 21:53 ` "tirthgajera (@tirthgajera)" <[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