Skip to content

Implement SQLGetStmtAttr and SQLSetStmtAttr - #57

Merged
rscales merged 20 commits into
apache-odbcfrom
sql-get-stmt-attr
Jul 14, 2025
Merged

Implement SQLGetStmtAttr and SQLSetStmtAttr#57
rscales merged 20 commits into
apache-odbcfrom
sql-get-stmt-attr

Conversation

@rscales

@rscales rscales commented Jul 3, 2025

Copy link
Copy Markdown

Implement SQLGetStmtAttr and SQLSetStmtAttr

Comment thread cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_statement.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_statement.cc Outdated
@rscales
rscales force-pushed the sql-get-stmt-attr branch from 04ee639 to e5cdd2b Compare July 9, 2025 19:11
@rscales
rscales marked this pull request as ready for review July 9, 2025 21:13

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The statement attributes that end with *_PTR are pointers for array values. So we should test that we are able to set them to valid values

Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
@alinaliBQ

alinaliBQ commented Jul 9, 2025

Copy link
Copy Markdown

Example for setting and getting a pointer value:
Pre-requisite: would need validateSetStmtAttr and validateGetStmtAttr helpers that accept pointer values.

dummy_array; // create an array. 
validateSetStmtAttr(this->stmt, <attribute_ptr>, dummy_array);

// Verify the address is the same
validateGetStmtAttr(this->stmt, <attribute_ptr>, dummy_array);

This example uses an array but some statement pointer attributes are for single values as well

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have few comments, and also need to fix the merge conflict and run the CI checks for C++ and Dev

Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Few more comments for code style

Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated
Comment thread cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc Outdated

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, just waiting for the CI check results

@alinaliBQ

Copy link
Copy Markdown

The checks have passed. The Conda failure is not related to our code

@rscales
rscales merged commit 079c4a0 into apache-odbc Jul 14, 2025
22 of 24 checks passed
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