Doubts about TriggerAdapter's ResultSet behaviour

50 views
Skip to first unread message

Lukas Eder

unread,
Mar 16, 2013, 11:38:06 AM3/16/13
to h2-da...@googlegroups.com
Hello,

On the jOOQ user group, I have recently been made aware of an issue with H2's TriggerAdapter behaviour (details can be seen here: https://groups.google.com/d/msg/jooq-user/rGyc7dq9hRs/vpAzbxYjisMJ).

According to the TriggerAdapter Javadoc, the ResultSet arguments are in fact "infinite", even if they hold only one record. It states:

    "ResultSet.next does not need to be called (and calling it has no effect; it will always return true)."


The general contract of a ResultSet doesn't seem to allow such behaviour. Specifically, the following is mentioned here:

    "ResultSet object maintains a cursor pointing to its current row of data. Initially the cursor is positioned before the first row. The next method moves the cursor to the next row, and because it returns false when there are no more rows in theResultSet object, it can be used in a while loop to iterate through the result set."

Taken from the first paragraphs of the JDBC Javadoc:

I understand that a previously initialised ResultSet seems convenient at first. But when interfacing with other APIs that consume such a ResultSet, "infinity" may cause weird effects. Wouldn't it be better to fix this behaviour and to avoid "unexpected" convenience?

Noel Grandin

unread,
Mar 16, 2013, 12:12:56 PM3/16/13
to h2-da...@googlegroups.com
TriggerAdapter is a special case. 

You are not supposed to be passing the object to anything else that consumes ResultSet's.

Yes, we could "fix" it by creating a custom interface, but that would break existing code.

Lukas Eder

unread,
Mar 17, 2013, 5:13:46 AM3/17/13
to h2-da...@googlegroups.com
I understand that a change would break existing code. Probably, a change could wait to the next major release.

I just wanted to stress the fact that I thought it was unwise to re-use an existing, well-defined contract (ResultSet), without implementing it correctly. Even if "you're not supposed to" pass the ResultSet instance to "anything else consuming it", well, client trigger code has to consume it somehow, and they're likely to consume it according to the general contract. So there's no escape :-)

Here are a couple of other SimpleResultSet methods, that do not work correctly in the context of a trigger:

Trigger ResultSets are updatable, but these methods return "default" values, which do not reflect that:

- getConcurrency()
- isReadOnly(int) (from ResultSetMetaData)
- isWritable(int) (from ResultSetMetaData)
- isDefinitelyWritable(int) (from ResultSetMetaData)

And these SimpleResultSet methods behave badly in any context:

- beforeFirst() works even if getType() is TYPE_FORWARD_ONLY. However, it doesn't work correctly, as a call to next() is required after a call to beforeFirst(), i.e. currentRow should be reset (at least).

It would probably be useful, if the type of the SimpleResultSet was changed to TYPE_SCROLL_INSENSITIVE, and then, if all the relevant methods, such as first(), last(), etc. were implemented. I can help with some implementations (e.g. making the SimpleResultSet TYPE_SCROLL_INSENSITIVE), if you agree with the above

Cheers
Lukas

Noel Grandin

unread,
Mar 17, 2013, 5:48:06 AM3/17/13
to h2-da...@googlegroups.com
TriggerAdapter is a __convenience__ interface.

Don't use it you don't like it, rather just use the Trigger interface directly.

Thomas Mueller

unread,
Mar 18, 2013, 2:14:04 PM3/18/13
to h2-da...@googlegroups.com
Hi,

I would like to keep the current behavior of the TriggerAdapter's ResultSet. The TriggerAdapter should be as convenient as possible (that's the reason to have it), and having to call next() isn't convenient. I would like to use the ResultSet interface because it's a known API to work with rows (there is no well known Row interface).

What we could do is throw an exception when trying to call next(). This also doesn't match the general contract of a ResultSet however. Or, return false when calling next().

Regards,
Thomas


On Sunday, March 17, 2013, Noel Grandin wrote:
TriggerAdapter is a __convenience__ interface.

Don't use it you don't like it, rather just use the Trigger interface directly.

--
You received this message because you are subscribed to the Google Groups "H2 Database" group.
To unsubscribe from this group and stop receiving emails from it, send an email to h2-database...@googlegroups.com.
To post to this group, send email to h2-da...@googlegroups.com.
Visit this group at http://groups.google.com/group/h2-database?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Lukas Eder

unread,
Mar 18, 2013, 2:49:13 PM3/18/13
to h2-da...@googlegroups.com
Hello,

I understand your arguments. Yes, the well-known Row interface is missing from the JDBC API (as are many other types). It would be the correct type to pass to a trigger. I wonder what would be required to form an expert group to finally fix all those JDBC API blunders...

Anyway, I will stop insisting :-)

Am Montag, 18. März 2013 19:14:04 UTC+1 schrieb Thomas Mueller:
Hi,

I would like to keep the current behavior of the TriggerAdapter's ResultSet. The TriggerAdapter should be as convenient as possible (that's the reason to have it), and having to call next() isn't convenient. I would like to use the ResultSet interface because it's a known API to work with rows (there is no well known Row interface).

What we could do is throw an exception when trying to call next(). This also doesn't match the general contract of a ResultSet however. Or, return false when calling next().

Regards,
Thomas


On Sunday, March 17, 2013, Noel Grandin wrote:
TriggerAdapter is a __convenience__ interface.

Don't use it you don't like it, rather just use the Trigger interface directly.

--
You received this message because you are subscribed to the Google Groups "H2 Database" group.
To unsubscribe from this group and stop receiving emails from it, send an email to h2-database+unsubscribe@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages