On Tue, 13 Oct 2026 07:06:28 GMT, Alan Bateman <[email protected]> wrote:
>> As the title suggests, permit `SyncProviderException.getSyncResolver()` to >> throw `IllegalStateException` when a sync resolver cannot be loaded. Note, >> there are no corresponding implementation changes in this PR. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProviderException.java > line 129: > >> 127: * <code>SyncResolver(String)</code> constructors are used to >> instantiate >> 128: * the <code>SyncResolver</code> instance. >> 129: * @throws IllegalStateException if a {@code SyncResolver} cannot >> be loaded. > > For the description then I think it would be clearer to say something "if a > SyncResolver is not set or the default implementation cannot be loaded". > SyncProviderException has a setter method so calling that out helps to > justify the ISE. Hi Alan, I thought we wanted this to only occur when there are no available implementations. i.e., no default and no third party implementations. If we throw `ISE` when no sync resolver has been set, it will be contradicting the specification, because it is specified to use the default impl when none has been set. * Retrieves the <code>SyncResolver</code> object that has been set for * this <code>SyncProviderException</code> object, or * if none has been set, an instance of the default <code>SyncResolver</code> * implementation included in the reference implementation. That is why we used "cannot be loaded" because it is consistent with `RowSetProvider.newFactory()` and only throws `ISE` when no impls are available, but retains the behavior to return the default if it exists and no sync resolver has been set. To be clear, I would be fine making the change you suggested, but just want to make sure we are on the same page. That change would then end up changing the behavior _even when the default implementation is available_. What do you think? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33329#discussion_r4261156920
