Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add way to distinguish base and partitioned tables in PgDatabaseMetaData.getTables #1708

Merged
merged 2 commits into from Feb 25, 2020

Conversation

MSGoodman
Copy link
Contributor

@MSGoodman MSGoodman commented Feb 18, 2020

There is currently no way to distinguish between base tables and
partitioned tables in the response from PgDatabadeMetaData.getTables.
Fix this by adding PARTITIONED TABLE to the tableTypeClauses map and
thus the TABLE_TYPE field in the query in getTables. Also update a test
(testPartitionedTables) that was using TABLE_TYPE of TABLE to grab
partitioned tables.

This should close #1590. However, perhaps this could be considered a
breaking change for anyone using getTables and expecting partitioned
tables to show up for .getTables(s, s1, s2, new String[]{"TABLE"})?

All Submissions:

  • [✓] Have you followed the guidelines in our Contributing document?
  • [✓] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. [✓] Does your submission pass tests?
  2. [✓] Does mvn checkstyle:check pass ?

Changes to Existing Features:

  • [✓] Does this break existing behaviour? If so please explain.

As above:

perhaps this could be considered a breaking change for anyone using getTables and expecting partitioned tables to show up for .getTables(s, s1, s2, new String[]{"TABLE"})?

  • [✓] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [✓] Have you successfully run tests with your changes locally?

…eMetaData.getTables

There is currently no way to distinguish between base tables and
partitioned tables in the response from PgDatabadeMetaData.getTables.
Fix this by adding PARTITIONED TABLE to the tableTypeClauses map and
thus the TABLE_TYPE field in the query in getTables. Also update a test
(testPartitionedTables) that was using TABLE_TYPE of TABLE to grab
partitioned tables.

This should close pgjdbc#1590. However, perhaps this could be considered a
breaking change for anyone using getTables and expecting partitioned
tables to show up for .getTables(s, s1, s2, new String[]{"TABLE"})?
@davecramer
Copy link
Member

davecramer commented Feb 18, 2020

We would also need to update

public ResultSet getTableTypes() throws SQLException {
and the associated tests although looking at the associated tests one is redundant and the other
public void testDatabaseMetaDataNames() throws SQLException {
doesn't test much

@MSGoodman
Copy link
Contributor Author

MSGoodman commented Feb 19, 2020

I am saddened that I missed these, thank you for letting me know!

I improved DatabaseMetaDataTest.testTableTypes() such that it now asserts that the returned table types are what we expect.

Unfortunately, I am unsure what there is to change about PgDatabaseMetaData.getTableTypes(), as it appears to work as expected?

As far as ResultSetMetaDataTest.testDatabaseMetaDataNames() is concerned, I'm unsure if this ought to be changed, as it seems as if it's just testing that the ResultSet has the correct column name, regardless of the actual values returned from PgDatabadeMetaData.getTableTypes()?

If there's anything I'm misunderstanding and should fix up then please let me know!

@codecov-io
Copy link

codecov-io commented Feb 19, 2020

Codecov Report

Merging #1708 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #1708      +/-   ##
============================================
+ Coverage     69.26%   69.31%   +0.04%     
- Complexity     4189     4195       +6     
============================================
  Files           187      187              
  Lines         17262    17266       +4     
  Branches       2870     2870              
============================================
+ Hits          11957    11968      +11     
+ Misses         4017     4015       -2     
+ Partials       1288     1283       -5

@davecramer
Copy link
Member

davecramer commented Feb 19, 2020

ah, I missed where you added it to tabletypes. Cool. Thx

@davecramer
Copy link
Member

davecramer commented Feb 19, 2020

So the only question now is regarding the breaking change. I would agree this should be a breaking change. Anyone else?

@davecramer davecramer modified the milestones: 42.3.0, 42.2.6 Feb 21, 2020
@davecramer davecramer merged commit 25eb32c into pgjdbc:master Feb 25, 2020
3 checks passed
gunnarmorling added a commit to gunnarmorling/debezium that referenced this issue Apr 29, 2020
The driver upgrade mitigates some issues with using this connector with
Postgres on Azure. It comes with some behavioural changes, though:

* column metadata for DECIMAL without scale is returned differently by
the (see pgjdbc/pgjdbc#1767): while it used
to be returned as 0, it's now returned as null. This should be
transparent to DBZ consumers
* snapshots for partitioned tables only export change events on the
partition topics now due to pgjdbc/pgjdbc#1708;
this has an impact on consumers, but I think it's more reasonable than
exporting all change events twice, one partition table and main table
topics
jpechane pushed a commit to debezium/debezium that referenced this issue Apr 29, 2020
The driver upgrade mitigates some issues with using this connector with
Postgres on Azure. It comes with some behavioural changes, though:

* column metadata for DECIMAL without scale is returned differently by
the (see pgjdbc/pgjdbc#1767): while it used
to be returned as 0, it's now returned as null. This should be
transparent to DBZ consumers
* snapshots for partitioned tables only export change events on the
partition topics now due to pgjdbc/pgjdbc#1708;
this has an impact on consumers, but I think it's more reasonable than
exporting all change events twice, one partition table and main table
topics
gunnarmorling added a commit to gunnarmorling/debezium that referenced this issue Jul 15, 2020
….12";

The driver upgrade mitigates some issues with using this connector with
Postgres on Azure. It comes with some behavioural changes, though:

* column metadata for DECIMAL without scale is returned differently by
the (see pgjdbc/pgjdbc#1767): while it used
to be returned as 0, it's now returned as null. This should be
transparent to DBZ consumers
* snapshots for partitioned tables only export change events on the
partition topics now due to pgjdbc/pgjdbc#1708;
this has an impact on consumers, but I think it's more reasonable than
exporting all change events twice, one partition table and main table
topics
jpechane pushed a commit to debezium/debezium that referenced this issue Jul 15, 2020
….12";

The driver upgrade mitigates some issues with using this connector with
Postgres on Azure. It comes with some behavioural changes, though:

* column metadata for DECIMAL without scale is returned differently by
the (see pgjdbc/pgjdbc#1767): while it used
to be returned as 0, it's now returned as null. This should be
transparent to DBZ consumers
* snapshots for partitioned tables only export change events on the
partition topics now due to pgjdbc/pgjdbc#1708;
this has an impact on consumers, but I think it's more reasonable than
exporting all change events twice, one partition table and main table
topics
davecramer pushed a commit to davecramer/pgjdbc that referenced this issue Jul 5, 2021
…eMetaData.getTables (pgjdbc#1708)

* feat: add way to distinguish base and partitioned tables in PgDatabaseMetaData.getTables

There is currently no way to distinguish between base tables and
partitioned tables in the response from PgDatabadeMetaData.getTables.
Fix this by adding PARTITIONED TABLE to the tableTypeClauses map and
thus the TABLE_TYPE field in the query in getTables. Also update a test
(testPartitionedTables) that was using TABLE_TYPE of TABLE to grab
partitioned tables.

This should close pgjdbc#1590. However, perhaps this could be considered a
breaking change for anyone using getTables and expecting partitioned
tables to show up for .getTables(s, s1, s2, new String[]{"TABLE"})?

* test: improve database metadata table type test
mdespriee added a commit to mdespriee/pgjdbc that referenced this issue Jul 29, 2021
tvainika added a commit to aiven/jdbc-connector-for-apache-kafka that referenced this issue Mar 3, 2022
Add handling for detecting existence of partitioned tables explicitly.

PostgreSQL JDBC Driver update from 42.2.10 to 42.2.25 in #113 caused
partitioned tables to broke due upstream change of separating types of
normal table and partitioned table as explained in
pgjdbc/pgjdbc#1708
tvainika added a commit to aiven/jdbc-connector-for-apache-kafka that referenced this issue Mar 3, 2022
Add handling for detecting existence of partitioned tables explicitly.

PostgreSQL JDBC Driver update from 42.2.10 to 42.2.25 in #113 caused
partitioned tables to broke due upstream change of separating types of
normal table and partitioned table as explained in
pgjdbc/pgjdbc#1708
Sign up for free to 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