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

Modified get column meta method to reference saved metadata #978

Merged
merged 3 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ env:
- TEST_PHP_SQL_PWD=Password123

before_install:
- docker pull mcr.microsoft.com/mssql/server:2017-latest
- docker pull mcr.microsoft.com/mssql/server:2019-CTP2.4-ubuntu

install:
- docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d mcr.microsoft.com/mssql/server:2017-latest
- docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d mcr.microsoft.com/mssql/server:2019-CTP2.4-ubuntu
- docker build --build-arg PHPSQLDIR=$PHPSQLDIR -t msphpsql-dev -f Dockerfile-msphpsql .

before_script:
Expand Down
12 changes: 5 additions & 7 deletions source/pdo_sqlsrv/pdo_stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ int pdo_sqlsrv_stmt_describe_col( _Inout_ pdo_stmt_t *stmt, _In_ int colno TSRML

// Set the name
column_data->name = zend_string_init( (const char*)core_meta_data->field_name.get(), core_meta_data->field_name_len, 0 );
core_meta_data->field_name.reset();

// Set the maxlen
column_data->maxlen = ( core_meta_data->field_precision > 0 ) ? core_meta_data->field_precision : core_meta_data->field_size;
Expand Down Expand Up @@ -1066,10 +1065,12 @@ int pdo_sqlsrv_stmt_get_col_meta( _Inout_ pdo_stmt_t *stmt, _In_ zend_long colno
// initialize the array to nothing, as PDO requires us to create it
core::sqlsrv_array_init( *driver_stmt, return_value TSRMLS_CC );

sqlsrv_malloc_auto_ptr<field_meta_data> core_meta_data;

core_meta_data = core_sqlsrv_field_metadata( driver_stmt, (SQLSMALLINT) colno TSRMLS_CC );
field_meta_data* core_meta_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we not using the smart pointer 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.

Because if we use smart pointer the entire data will be transferred. See sqlsrv_malloc_auto_ptr definition


// metadata should have been saved earlier
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know the metadata is always saved before it gets to this point? Is pdo_sqlsrv_stmt_describe_col for sure called before pdo_sqlsrv_stmt_get_col_meta?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, see pdo_stmt_describe_columns in pdo_stmt.c

SQLSRV_ASSERT(colno < driver_stmt->current_meta_data.size(), "pdo_sqlsrv_stmt_get_col_meta: Metadata vector out of sync with column numbers");
core_meta_data = driver_stmt->current_meta_data[colno];

// add the following fields: flags, native_type, driver:decl_type, table
add_assoc_long( return_value, "flags", 0 );

Expand Down Expand Up @@ -1109,9 +1110,6 @@ int pdo_sqlsrv_stmt_get_col_meta( _Inout_ pdo_stmt_t *stmt, _In_ zend_long colno
if( stmt->columns && stmt->columns[colno].param_type == PDO_PARAM_ZVAL ) {
add_assoc_long( return_value, "pdo_type", pdo_type );
}

// this will ensure that the field_name field, which is an auto pointer gets freed.
(*core_meta_data).~field_meta_data();
}
catch( core::CoreException& ) {

Expand Down