Mapping Exception when Stored Procedure exits prior to performing select

[GoogleCodeExporter]

If a Stored Procedure never actually executes a select query to return a result 
set because a stored procedure short circuits (due to conditions in the sproc) 
an ArgumentException is thrown "When using the multi-mapping APIs ensure you 
set the splitOn param if you have keys other than Id""

Attached is a stored procedure that does this, the business case is to check 
user permissions at the database level instead of the application level.

pseudo/example code to test:

******
    private class myInt
    {
        public int val { get; set; }
    }
private void ServerCall() {
  var con = ConnectionFactory.GetConnection();
  var p = new DynamicParameters();
  p.Add("retcode", dbType: DbType.Int32, direction: ParameterDirection.Output);
  p.Add("earlyExit", true, DbType.Boolean);
  var val = con.Query<myInt>("z_SP_TEST_EARLY_RETURN_SINGLE", p, commandType:CommandType.StoredProcedure).SingleOrDefault();
}

******

I've resolved this locally by modifying the code in GetClassDeserializer<T> to 
validate that reader.FieldCount is != 0 before throwing the exception

e.g.:  if (reader.FieldCount <= startBound && reader.FieldCount != 0) 

Thoughts/Comments?

Original issue reported on code.google.com by tkarpin...@gmail.com on 1 Aug 2011 at 5:59

Attachments:

• samplesproc.sql

[GoogleCodeExporter]

If your query doesn't perform a SELECT, then Execute is preferable to Query. 
QueryMultiple may also be useful; however, in most cases performing Query and 
not getting a results grid is an error. The best "fix" I can propose here is a 
clearer exception message. Is that what you are looking for?

Original comment by marc.gravell on 4 Aug 2011 at 6:18

[GoogleCodeExporter]

Hi Mark,

I completely agree that if it doesn't perform a select that it should use 
Execute, but in our case it is conditional, the stored proc validates a passed 
in user token prior to performing a select, if the token does not have rights 
then the select does not occur.  Obviously this is specific to my usage but I 
am curious whether my suggested change would have any negative consequences to 
the rest of dapper.

Original comment by tkarpin...@gmail.com on 4 Aug 2011 at 12:39

[GoogleCodeExporter]

I just ran into this by accident, and I think it makes a good example of why 
this should get fixed. 

I had a bug in my sproc where it wasn't running the SELECT. Using 
con.Query<MyObj> I was getting the exception message about multi-mapping, and 
this completely threw me off. I started debugging through the dapper source to 
see where this was coming from, noticed the fields returned were 0, and then 
quickly found the problem in my sproc. 

If dapper had returned either an empty list of MyObj's (which I believe is what 
@tkarpin...'s change would do), or thrown an exception to the effect of "0 
fields returned" (as per @marc.gravell), I would have found and resolved the 
issue with my sproc in a matter of seconds rather than the comparably huge 
amount of time to get dapper source loaded in a debugger, attach and step 
through, and then make sense of what was going on.

Original comment by gro...@gmail.com on 15 Mar 2012 at 5:48

[GoogleCodeExporter]

I've hit a related issue:

If the first execution of a query returns an empty result set, all subsequent 
executions of the same query fail within Dapper even if they return rows. 
Traced this down to the cached deserializer being built before checking there's 
a record in the IDataReader.

Applied a simple fix, whereby the deserializer only gets built if missing and 
when needed:

Within QueryInternal<T>:

                    var deserializer = info.Deserializer;

                    while (reader.Read())
                    {
                        if (deserializer == null) // only try to cache once we've got something to deserialize
                        {
                            deserializer = cacheDeserializer();
                        }
                        object next;
                        try
                        {
                            next = deserializer(reader);
                        }
                        catch (DataException)
                        {
                            // give it another shot, in case the underlying schema changed
                            deserializer = cacheDeserializer();
                            next = deserializer(reader);
                        }
                        yield return (T)next;
                    }

Original comment by Dave.Hud...@gmail.com on 15 Jun 2012 at 1:08

[GoogleCodeExporter]

The "fails first time, then you're knackered" needs fixing; I'll take that. 
Note I think your fix is based on an old version of the code, but I can tweak 
it from there, thanks.

Original comment by marc.gravell on 15 Jun 2012 at 1:38

[GoogleCodeExporter]

Marc, just looked at GitHub, don't see anything that looks like your update.  
Did it make it in, or if not when might we expect it? Thanks.

Original comment by geoffrey...@gmail.com on 28 Jun 2012 at 4:57

[GoogleCodeExporter]

I have the same issue as [@tkarpin] with conditional SELECT, when SQL statement 
is not always returning result set.
As well, would be good to have some flag in GridReader set (some where in 
NextResult()) when to result sets are available.

Original comment by givas...@gmail.com on 2 Jun 2013 at 3:41

[GoogleCodeExporter]

I have the same issue with Dapper 1.13... Any word on this? Seems like a simple 
fix. I'm happy to send a pull-request if there's any sign that this project is 
still alive (doesn't look like it).

Original comment by Caio.Proiete on 31 Jul 2013 at 12:41