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

Add ODBC DirectExec public API #3502

Merged
merged 8 commits into from Mar 22, 2023

Conversation

Jihadist
Copy link
Contributor

@Jihadist Jihadist commented Jan 24, 2022

It was a very long story how i couldn't determine why temp tables dont work in Poco between statements (#3499). After all, I compared how Poco use ODBC api and NanoODBC
For 1 single statement Poco call stack (except attrs and some non-related things):

SQLAllocHandle
SQLPrepareW
SQLNumResultCol
SQLExecute
SQLFetch
SQLRowCount
SQLFreeHandle

For 1 single statement NanoODBC call stack (except attrs and some non-related things):

SQLAllocHandle
SQLExecDirect
SQLNumResultCol
SQLCancel
SQLFreeStmt
SQLFreeHandle

Looks like Poco uses only prepared statement while nanodbc calls execute direct every time. I tried to investigate deeper and deeper. If I create temp table with prepared statement, temp table will not be available in the next statement. How it works with sessions but with statements. But if I create temp table with direct execute, it will be available in the statements...
So I tried to add direct execute to Poco but I dont understand well Poco architecture and it needs to change a lot of classes and maybe architecture too. So I create this PR as proposal or draft.
If you provide me some help I'll be very thankful.

Closes #3499

MS mention this in docs https://learn.microsoft.com/en-us/sql/relational-databases/native-client-odbc-queries/executing-statements/prepared-execution?view=sql-server-ver16
Prepared statements cannot be used to create temporary objects on SQL Server.

auto data=&query.at(0);
SQLCHAR * statementText=reinterpret_cast<unsigned char*>(data);
SQLINTEGER textLength=query.size();
SQLRETURN rc = SQLExecDirect(_stmt,statementText,textLength);
Copy link

@dedlocc dedlocc Jan 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Add spaces around = and after commas.
  2. Better use query.data() instead of &query.at(0) and then there's no need of a separate variable.
  3. Even though it seems you do need to pass a non-const char pointer to SQLExecDirect, you don't need it for every possible implementation of this method (e.g. in StatementImpl::execDirectImpl). So I'd suggest passing it as const reference and only make a copy if needed.
  4. * should be a part of type (so no whitespace before it), as suggested here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data/include/Poco/Data/StatementImpl.h Outdated Show resolved Hide resolved
Data/include/Poco/Data/StatementImpl.h Show resolved Hide resolved
Data/include/Poco/Data/Statement.h Outdated Show resolved Hide resolved
Data/src/Statement.cpp Outdated Show resolved Hide resolved
@Jihadist Jihadist force-pushed the feature/odbc-exec-direct branch 2 times, most recently from 2039682 to 32949be Compare January 24, 2022 14:49
@obiltschnig obiltschnig added this to the Release 1.12.0 milestone Feb 11, 2022
@Jihadist
Copy link
Contributor Author

Jihadist commented Oct 8, 2022

@aleks-f sorry for ping, but are there any chances for this pr?

@aleks-f
Copy link
Member

aleks-f commented Mar 17, 2023

@Jihadist yes, but there are still some unanswered questions above

@Jihadist
Copy link
Contributor Author

@Jihadist yes, but there are still some unanswered questions above

If you mean async part, I don't know how to implement it. Maybe you can suggest something

@aleks-f
Copy link
Member

aleks-f commented Mar 18, 2023

@Jihadist yes, but there are still some unanswered questions above

If you mean async part, I don't know how to implement it. Maybe you can suggest something

I did suggest something, you just have to read it

image

@Jihadist
Copy link
Contributor Author

@aleks-f it sounds strange, but I don't see anything
изображение

Data/src/Statement.cpp Show resolved Hide resolved
Data/ODBC/testsuite/src/ODBCSQLServerTest.cpp Show resolved Hide resolved
@aleks-f aleks-f merged commit 93d1816 into pocoproject:devel Mar 22, 2023
15 checks passed
aleks-f pushed a commit that referenced this pull request Nov 27, 2023
* ODBC sqlDirectExec

* doc

* Small fix

* Fix tabs, add missing const, fix style

* Add test case

* Small fixes

* suggested fix for async

* test for returned values
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.

ODBC Driver: temp tables
5 participants