Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the apparently bogus "FALSE &&" condition in SC_forget_unnamed(). #1

Open
wants to merge 1,065 commits into
base: master
Choose a base branch
from

Conversation

vadz
Copy link

@vadz vadz commented Apr 12, 2013

This was added, perhaps accidentally, in 5f35a04, and resulted in no results
being returned from SQLFetch() for prepared statements.

h-saito and others added 30 commits December 28, 2007 14:42
Per report Stephen M. Lathrop.
Per report Stephen M. Lathrop.
2. Support column alias without "as" so that links from
   the SQLServer work.
3. Take ';' into account when the driver adds "for read only"
   clause.
4. Use the E'.. ' notation not only in '=' expressions but
   also in LIKE expressions.
5. Change to return milliseconds parts for timestamp fields.
6. Change to return a specific sqlstate in case of multiple
   parameters.
(PostgreSQL Version 8.3 of MSVC)
1. SQLGUID type support thanks to Jan-Willem Goossens.
2. Fix a bug about silently adding a *for read only* clause.
3. Fix a 64bit mode bug about handling of arrays of parameters.
4. Change the implemetatin of SQLForeignKeys() for 8.3+ servers.
5. Not commit the transaction too early in useDeclareFetch mode.
6. Add a cursor open check for SQLPrepare().
1. Reset the column binding information after SQLMoreResults().
2. Save the rowset size properly for the FETCH_NEXT operation
   in case of >= 3.0 drivers.

Enable geqo optiomizer by default.
2. Check strerror_r function's return type.
3. Suppress some compiler warnings.
hlinnaka and others added 29 commits February 22, 2013 11:13
We liberally assign between SQLCHAR * and char *, which generates a lot of
compiler warnings about different pointer signedness. Seems like a lost
cause to add casts everywhere (and it might not be good for robustness
anyway, because it might mask a bug where the datatype of one side is
changed to a struct or something in the future), so just silence the
warnings.
Silences a compiler warning.
For some reason, the destructor was not being called when compiled with gcc.
Not sure why, but I'm assuming it was an oversight. This silences a compiler
warning complaining that finalize_global_cs() function was unused.
BTW, the error and notice handling could be made much smarter wrt.
truncation. The caller of handle_notice/error_message knows the message size
in advance, so it could easily pass it down and we could allocate a buffer
of the right size, instead of using a fixed size buffer that can be too
small.
If realloc fails and returns NULL, don't forget the old array. Also check
for overflow of num_Stmts, which is only a 16-bit int
We reference the same pointer without null-check elsewhere in the function,
so if it's ever NULL, we'll crash anyway. (but looking at the callers, it
never is NULL)

Found by Coverity scan.
The problem is that if the loop above doesn't find a result set with
command tag "FETCH ...", res is left to point to an already-freed result
set, and we'll go ahead and call QR_set_withhold on the already-freed
struct. That can't happen in practice, because the backend does always
return a result set with "FETCH ..." command tag when a FETCH statement is
issued, but better safe than sorry.
Found by Coverity scan.
Coverity doesn't like the fact that the struct is passed to
SC_set_rowset_start without initializing rowset_start field. It's OK,
SC_set_rowset_start doesn't actually use the 'incr' value calculated from
the field if the 3rd arg is FALSE, so this is a false positive, but seems
like a good idea to be tidy here anyway.
This avoids accessing unitialized bitmap[0]. It was harmless, as the
uninitialize value that was read was not used for anything, but Coverity
complained about it.
It's a good habit to mark constants as such. But I'm also hoping that this
would silence Coverity from complaining that ver2str and ver3str might
not fit in 6 bytes.
It wasn't actually true, but apparently not all compiler versions were smart
enough to realize it.
To silence a Coverity warning that the semicolon might be misplaced.
If you have a lot of chained results, e.g. if you array bind a lot of
parameters, recursing through the whole chain might make you run out of
stack space. Convert recursion to iteration.

Also, when we're freeing all the results in the chain, we only need to
call QR_set_cursor(self, NULL) once, on the first result in the chain.
QR_set_cursor itself loops through the whole chain, resetting the cursor
name of every result. By only calling it for the first result, we avoid
O(n^2) behavior, significantly speeding up SQLFreeHandle() on a statement
with a large number of chained results.
There was at least one real bug caused by the multiple evaluation of 2nd
argument to SC_set_result(). SC_pre_execute() passed QR_Constructor() call
as the 2nd argument, so we ended up creating extra result sets that were
leaked.
…depaths.

This patch eliminates O(n^2) behavior on the number of chained results in
SC_execute. This makes it feasible to deal with large parameter arrays, for
example.

The idea is to track the owner statement of each result object, so that when
we need to check if a result object is already in the chain of results of a
statement, we just need to look at the owner field, not loop through the
whole chain. Also, keep track of the last result in the chain in
StatementClass, so that we don't need to iterate through it when appending
to the end.
If a query was sent to the server using the Parse command, we returned the
dummy QResult for the Parse, with 0 rows, to the caller of SQLExecute.
Fix by clearing out the result set just before executing.
…wrong.

With the bogus fix, we only returned the last result set for an array bound
statement. Move resetting the result set to before the next_param_row label,
so that it's only done once when using a parameter array.
This was added, perhaps accidentally, in 5f35a04, and resulted in no results
being returned from SQLFetch() for prepared statements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants