Row.decode(…) fails for enum type with IllegalArgumentException: 72093 is not a valid object id

[mp911de]

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open-source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution issues. Thanks!

Problem

AbstractTemporalCodec.canDecode fails as it tries to create a PostgresqlObjectId to verify whether it can decode a timestamp. OID 72093 is a custom type that isn't known by PostgresqlObjectId and AbstractTemporalCodec.canDecode should not fail, it should rather report that it cannot decode the value by returning false.

As user, calling Row.get(…) leads to:

java.lang.IllegalArgumentException: 72093 is not a valid object id
	at io.r2dbc.postgresql.type.PostgresqlObjectId.valueOf(PostgresqlObjectId.java:417)
	at io.r2dbc.postgresql.codec.AbstractTemporalCodec.canDecode(AbstractTemporalCodec.java:69)
	at io.r2dbc.postgresql.codec.DefaultCodecs.decode(DefaultCodecs.java:128)
	at io.r2dbc.postgresql.PostgresqlRow.decode(PostgresqlRow.java:90)
	at io.r2dbc.postgresql.PostgresqlRow.get(PostgresqlRow.java:77)
	at io.r2dbc.spi.Row.get(Row.java:78)

Solution

AbstractTemporalCodec.canDecode(…) should make use of PostgresqlObjectId.isValid(dataType) before looking up the default type from PostgresqlObjectId. Other codecs such as AbstractCodec are performing the check correctly.

Steps to Fix

• Claim this issue with a comment below and ask any clarifying questions you need
• Set up a repository locally following the Contributing Guidelines
• Try to fix the issue following the steps above
• Commit your changes and start a pull request.

[AlbertMMM]

@mp911de I would like to claim this issue. I have the the repository locally set up and I'm able to run the tests. Can you please point me in the right direction to mocking As user, calling Row.get(…)?

[mp911de]

You're welcome @AlbertMMM. I would suggest starting with a test for canDecode using a subclass of AbstractTemporalCodec, e.g. InstantCodec. Codecs are used from PostgresqlRow. However mocking PostgresqlRow along with all the required objects (column descriptors, row data) would impose a lot of work and it would be a test with many directions. Therefore, I suggest starting on the codec level directly.

Let us know if you need any further details.

[mp911de]

Since the submitted pull request had to be closed due to inactivity, this ticket is up for grabs again.

[govi20]

Hi, I would like to give it a shot.

[mp911de]

You're welcome @govi20.

[govi20]

@mp911de Thanks, Mark. How do I test it? could you please explain so that I can write test cases.

[mp911de]

Take a look at an earlier comment: #301 (comment). Let me know if that helps or your need further guidance. Happy to help.

[uazw]

Hi, I just raise a pr based on the conversion, can you please have a look?

[nefsir]

Hi! I have same issue using HStore data type. Both AbstractTemporalCodec and AbstractNumericCodec need to return false instead throwing an exception.

[mp911de]

@uazw this is a bit unfortunate as @govi20 claimed to work on the issue and it would not be fair towards @govi20 who wanted to spend time.

[uazw]

@mp911de sure thing! let me close my pr first, and looking for others issues.

[mp911de]

Let's wait for a response from @govi20 before we proceed.

[hvicha]

Hello All, there is same bug in AbstractNumericCodec.canDecode(…)

[govi20]

Thanks for pointing it out. Fixed it locally. @mp911de Should I push it on the same branch?

[mp911de]

Yes, please. Please also note that I'll be back on Aug 31.