dolt icon indicating copy to clipboard operation
dolt copied to clipboard

Add support for handlers

Open tim-oh-thee opened this issue 1 year ago • 15 comments

The documentation here: https://docs.dolthub.com/sql-reference/sql-support/supported-statements states that handlers are only supported for cursors. Could we add support for these? I was recently trying to create a procedure and handle potential errors and didn't really have an option to do it.

MySQL reference: https://dev.mysql.com/doc/refman/8.0/en/handler.html

tim-oh-thee avatar Feb 02 '24 20:02 tim-oh-thee

Hey @tim-oh-thee,

Just wanted to make sure we are on the same page. You want support for the other where_conditions in DECALRE ... HANDLER ...: https://dev.mysql.com/doc/refman/8.0/en/declare-handler.html

Not support for the Handler statement: https://dev.mysql.com/doc/refman/8.0/en/handler.html

jycor avatar Feb 03 '24 00:02 jycor

Yep you’re absolutely right. My apologies for any confusion.

tim-oh-thee avatar Feb 03 '24 01:02 tim-oh-thee

Hi @tim-oh-thee, would you be willing to share the specific behavior you are interested in? We don't meaningfully support handler execution right now and there are a lot of different features. Most condition filtering is based on JDBC sql state codes, and we also do not support those currently.

For example, I can see a path towards supporting specific SQL error code integers because that is independent of state codes. If you need state code support I would need to do more research to figure out how to add that separate layer.

max-hoffman avatar Feb 05 '24 21:02 max-hoffman

Aha that might be the right place to start. I was trying to do that exact thing. I forgot that I was coming up with the same sql state no matter what. The procedure I was writing did not need to be that robust so I was fine with just catching the any exception and skipping it. So, in this case the fact that I couldn't use a handler to do that was the limitation I was hitting, not the supported SQL state codes.

tim-oh-thee avatar Feb 05 '24 21:02 tim-oh-thee

The way the current code works is that you can only create NOT FOUND condition handlers. But we do not perform any condition filter, so the handlers will run on all errors (@Hydrocharged let me know if I'm misunderstanding). A thing we could do is let you create every type of condition but continue to not do error filtering. The handlers would run in inappropriate circumstances in that case, but it would let you skip errors with the proper procedure syntax. In the future, if you needed to distinguish between certain types of errors we could come back and implement filters for certain error integers, or add sql state codes if need-be. Does that sound like a useful progression for you?

max-hoffman avatar Feb 05 '24 22:02 max-hoffman

Want to make sure I'm understanding this correctly. Essentially you'll be providing me the capability to say: DECLARE CONTINUE HANDLER FOR 1051 BEGIN -- body of handler END;

but since there is no granularity in the error messages, I won't be able to control whether that handler is working on a specific issue (for example only an error for when a table doesn't exist). It'd essentially execute the body of the handler for every error.

If this understanding is correct, then this makes sense to me and fits my intent with what I was trying to do in the first place. It'd be nice to see this built out more in the future for different error codes or sql states, etc..

tim-oh-thee avatar Feb 05 '24 23:02 tim-oh-thee

Did a deeper pass and I think it's going to be more involved than I originally thought. Do you have any codes other than 1051 that you would like to support? We might need to do these one by one, but there are thousands of errors to map and so it will be most practical to target a subset on the first iteration.

max-hoffman avatar Feb 06 '24 00:02 max-hoffman

For context, one of the problems here is that we do not have a 3-way mapping between sql states, error codes, and Dolt error types. So we can go and catalog all of our error types, but there are thousands of relationships we do have and probably more that we do not or need to be refactored to support full sql state partitioning.

The second problem is adding callbacks at any point where errors happen in stored procedure execution to perform the filtering and handler check. The first version of this feature only implemented the NOT FOUND error because it only happens in LOOP calls and doesn't require any sql state normalization.

The space of handler behavior is big and I'd prefer unblock you soon rather than spending a month trying to hit every error code. There are certain common errors that could start cataloging and releasing first -- "table not found", "column not found", "syntax error", procedure specific errors, ...etc. But I am also not the stored procedure expert and guidance on your use case would help us make sure we're fixing what you need.

max-hoffman avatar Feb 06 '24 16:02 max-hoffman

Could we do the SQLEXCEPTION one? DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN -- body of handler END;

tim-oh-thee avatar Feb 07 '24 14:02 tim-oh-thee

1051 was just the example I pulled from the documentation, nothing important about that one specifically.

tim-oh-thee avatar Feb 07 '24 14:02 tim-oh-thee

Yeah I was thinking the same thing last night, I'll start adding SQLEXCEPTION support this morning. The work for that feature should make it easier to quickly extend support for specific codes in the future. Thank you for the clarifications @tim-oh-thee !

max-hoffman avatar Feb 07 '24 15:02 max-hoffman

Thanks for the help!

tim-oh-thee avatar Feb 07 '24 15:02 tim-oh-thee

Thanks for your patience @tim-oh-thee, I have a PR here that id some directional progress that you might find useful: https://github.com/dolthub/go-mysql-server/pull/2323.

This should support SQLEXCEPTION handlers that run for errors (other than those caught by NOT FOUND, same behavior as MySQL). The setup there should let us extend support for specific error codes or states in the future. And of course if the feature has correctness gaps compared to MySQL let us know and we will follow up and fix ASAP.

We will let you know when this PR makes it into a Dolt release.

max-hoffman avatar Feb 08 '24 21:02 max-hoffman

Perfect, thank you Max!

tim-oh-thee avatar Feb 09 '24 18:02 tim-oh-thee

The changes should be in dolt 1.34.0. I am going to leave this thread open for visibility given the limited support currently.

max-hoffman avatar Feb 10 '24 01:02 max-hoffman