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

OdbcResult: Ability to explicitely describe parameters #863

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ S3method(odbcListColumns,OdbcConnection)
S3method(odbcListObjectTypes,default)
S3method(odbcListObjects,OdbcConnection)
S3method(odbcPreviewObject,OdbcConnection)
export(ODBC_TYPE)
export(databricks)
export(isTempTable)
export(odbc)
Expand Down
18 changes: 18 additions & 0 deletions R/RcppExports.R
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ bigint_mappings <- function() {
.Call(`_odbc_bigint_mappings`)
}

#' Access the definition of the named SQL Data Type.
#'
#' @description Helper method returning the integer of the
#' [SQL Data Type](https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/sql-data-types?view=sql-server-ver16)
#' named in the argument.
#'
#' @seealso <https://github.com/microsoft/ODBC-Specification/blob/master/Windows/inc/sql.h>
#' @rdname odbcDataType.
#' @examples
#' \dontrun{
#' library(odbc)
#' ODBC_TYPE("LONGVARCHAR")
#' }
#' @export
ODBC_TYPE <- function(type) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, just to situate more comfily in this package's namespace, this may be better formatted as:

Suggested change
ODBC_TYPE <- function(type) {
odbcType <- function(type) {

.Call(`_odbc_ODBC_TYPE`, type)
}

result_release <- function(r) {
invisible(.Call(`_odbc_result_release`, r))
}
Expand Down
39 changes: 38 additions & 1 deletion R/dbi-result.R
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,54 @@ setMethod("dbGetRowsAffected", "OdbcResult",
}
)

#' @param param_description A data.frame with per-parameter attribute
#' overrides. Argument is optional; if used it must have columns:
#' * param_index Index of parameter in query ( beginning with 1 ).
#' * data_type Integer corresponding to the parameter SQL Data Type.
#' See \code{\link{ODBC_TYPE}}.
#' * column_size Size of parameter.
#' * decimal_digits Either precision or the scale of the parameter
#' depending on type.
Comment on lines +129 to +136
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' @param param_description A data.frame with per-parameter attribute
#' overrides. Argument is optional; if used it must have columns:
#' * param_index Index of parameter in query ( beginning with 1 ).
#' * data_type Integer corresponding to the parameter SQL Data Type.
#' See \code{\link{ODBC_TYPE}}.
#' * column_size Size of parameter.
#' * decimal_digits Either precision or the scale of the parameter
#' depending on type.
#' @param param_description A data frame containing per-parameter attribute overrides.
#' Optional argument that, if provided, must contain the following columns:
#' \describe{
#' \item{param_index}{Index of parameter in query (beginning with 1).}
#' \item{data_type}{Integer corresponding to the parameter SQL Data Type.
#' See [ODBC_TYPE()].
#' \item{column_size}{Size of parameter.}
#' \item{decimal_digits}{Either precision or the scale of the parameter
#' depending on type.}
#' }
  • Transitions to \describe for data frame columns
  • Links to ODBC_TYPE() with roxygen shorthand
  • Refers to "data frame" rather than data.frame since tibbles and other data.frame subclasses are fine

#'
#' For more information see the [ODBC SQLBindParam documentation](https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlbindparameter-function?view=sql-server-ver16)
#'
#' @examples
#' \dontrun{
#' library(odbc)
#' # Writing UNICODE into a VARCHAR
#' # column with SQL server
#' DBI::dbRemoveTable(conn, "#tmp")
#' dbExecute(conn, "CREATE TABLE #tmp (col1 VARCHAR(50) COLLATE Latin1_General_100_CI_AI_SC_UTF8);")
#' res <- dbSendQuery(conn, "INSERT INTO #tmp SELECT ? COLLATE Latin1_General_100_CI_AI_SC_UTF8")
#' description <- data.frame(param_index = 1, data_type = ODBC_TYPE("WVARCHAR"),
#' column_size = 5, decimal_digits = NA_integer_)
#' dbBind(res, params = list("\u2915"), param_description = description)
#' dbClearResult(res)
#' DBI::dbReadTable(conn, "#tmp")
#' }
Comment on lines +140 to +153
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' @examples
#' \dontrun{
#' library(odbc)
#' # Writing UNICODE into a VARCHAR
#' # column with SQL server
#' DBI::dbRemoveTable(conn, "#tmp")
#' dbExecute(conn, "CREATE TABLE #tmp (col1 VARCHAR(50) COLLATE Latin1_General_100_CI_AI_SC_UTF8);")
#' res <- dbSendQuery(conn, "INSERT INTO #tmp SELECT ? COLLATE Latin1_General_100_CI_AI_SC_UTF8")
#' description <- data.frame(param_index = 1, data_type = ODBC_TYPE("WVARCHAR"),
#' column_size = 5, decimal_digits = NA_integer_)
#' dbBind(res, params = list("\u2915"), param_description = description)
#' dbClearResult(res)
#' DBI::dbReadTable(conn, "#tmp")
#' }
#' @examplesIf FALSE
#' # Writing UNICODE into a VARCHAR column with SQL server
#' dbRemoveTable(conn, "#tmp")
#'
#' dbExecute(
#' conn,
#' "CREATE TABLE #tmp (col1 VARCHAR(50) COLLATE Latin1_General_100_CI_AI_SC_UTF8);"
#' )
#'
#' res <- dbSendQuery(
#' conn,
#' "INSERT INTO #tmp SELECT ? COLLATE Latin1_General_100_CI_AI_SC_UTF8"
#' )
#'
#' description <- data.frame(
#' param_index = 1,
#' data_type = ODBC_TYPE("WVARCHAR"),
#' column_size = 5,
#' decimal_digits = NA_integer_
#' )
#'
#' dbBind(res, params = list("\u2915"), param_description = description)
#' dbClearResult(res)
#'
#' dbReadTable(conn, "#tmp")

Some style edits I'd make:

  • examples with \dontrun{} -> examplesIf FALSE so that users don't see ## Not run tags and CRAN definitely won't try to run the examples
  • I believe the DBI:: namespacing here may not be needed?
  • Line-breaking more liberally via codegrip

#' @rdname OdbcResult
#' @inheritParams DBI::dbBind
#' @inheritParams DBI-tables
#' @export
setMethod("dbBind", "OdbcResult",
function(res, params, ..., batch_rows = getOption("odbc.batch_rows", NA)) {
function(res, params, ...,
param_description = data.frame(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
param_description = data.frame(),
param_description = NULL,

NULL indicates a bit more clearly to me that "this is the default that won't actually be used if you don't supply anything"

batch_rows = getOption("odbc.batch_rows", NA)) {
params <- as.list(params)
if (length(params) == 0) {
return(invisible(res))
}

if (nrow(param_description)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (nrow(param_description)) {
if (!is.null(param_description)) {

Aligning with above.

if (!all(c("param_index", "data_type", "column_size", "decimal_digits")
%in% colnames(param_description))) {
cli::cli_abort(
"param_description data.frame does not have necessary columns."
)
}
Comment on lines +168 to +173
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!all(c("param_index", "data_type", "column_size", "decimal_digits")
%in% colnames(param_description))) {
cli::cli_abort(
"param_description data.frame does not have necessary columns."
)
}
check_data_frame(param_description)
needed_columns <- c("param_index", "data_type", "column_size", "decimal_digits")
if (!all(needed_columns %in% colnames(param_description))) {
cli::cli_abort(
"{.arg param_description} must have columns {.field {needed_columns}}, but
doesn't have column{?s}
{.field {.or {needed_columns[needed_columns %in% colnames(param_description)]}}}."
)
}

check_data_frame() will first ensure that the thing is a data frame and provide an informative message if not before we do the finer-grain check for needed columns.

result_describe_parameters(res@ptr, param_description)
}

if (is.na(batch_rows)) {
batch_rows <- length(params[[1]])
}
Expand Down
36 changes: 35 additions & 1 deletion man/OdbcResult.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Makevars.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ PKG_CPPFLAGS=@PKG_CFLAGS@
PKG_CXXFLAGS=-Icctz/include -Inanodbc -I. -DBUILD_REAL_64_BIT_MODE -DNANODBC_ODBC_VERSION=SQL_OV_ODBC3 $(CXXPICFLAGS)
PKG_LIBS=@PKG_LIBS@ -Lcctz -lcctz

OBJECTS = odbc_result.o connection.o nanodbc.o result.o odbc_connection.o RcppExports.o Iconv.o utils.o
OBJECTS = odbc_types.o odbc_result.o connection.o nanodbc.o result.o odbc_connection.o RcppExports.o Iconv.o utils.o

all: $(SHLIB)

Expand Down
2 changes: 1 addition & 1 deletion src/Makevars.win
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PKG_CXXFLAGS=-I. -Icctz/include -Inanodbc
PKG_LIBS=-lodbc32 -Lcctz -lcctz

OBJECTS = odbc_result.o connection.o nanodbc.o result.o odbc_connection.o RcppExports.o Iconv.o utils.o
OBJECTS = odbc_types.o odbc_result.o connection.o nanodbc.o result.o odbc_connection.o RcppExports.o Iconv.o utils.o

all: $(SHLIB)

Expand Down
12 changes: 12 additions & 0 deletions src/RcppExports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,17 @@ BEGIN_RCPP
return rcpp_result_gen;
END_RCPP
}
// ODBC_TYPE
int ODBC_TYPE(const std::string& type);
RcppExport SEXP _odbc_ODBC_TYPE(SEXP typeSEXP) {
BEGIN_RCPP
Rcpp::RObject rcpp_result_gen;
Rcpp::RNGScope rcpp_rngScope_gen;
Rcpp::traits::input_parameter< const std::string& >::type type(typeSEXP);
rcpp_result_gen = Rcpp::wrap(ODBC_TYPE(type));
return rcpp_result_gen;
END_RCPP
}
// result_release
void result_release(result_ptr r);
RcppExport SEXP _odbc_result_release(SEXP rSEXP) {
Expand Down Expand Up @@ -384,6 +395,7 @@ static const R_CallMethodDef CallEntries[] = {
{"_odbc_transactionLevels", (DL_FUNC) &_odbc_transactionLevels, 0},
{"_odbc_set_transaction_isolation", (DL_FUNC) &_odbc_set_transaction_isolation, 2},
{"_odbc_bigint_mappings", (DL_FUNC) &_odbc_bigint_mappings, 0},
{"_odbc_ODBC_TYPE", (DL_FUNC) &_odbc_ODBC_TYPE, 1},
{"_odbc_result_release", (DL_FUNC) &_odbc_result_release, 1},
{"_odbc_result_active", (DL_FUNC) &_odbc_result_active, 1},
{"_odbc_result_completed", (DL_FUNC) &_odbc_result_completed, 1},
Expand Down
67 changes: 67 additions & 0 deletions src/odbc_types.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#include <unordered_map>
#include <string>
#include <sql.h>
#include <sqlext.h>
#include <sqlucode.h>
#include "utils.h"
#define INSTANTIATE_SQL_DATA_TYPE(TYPE) _the_map[#TYPE] = SQL_##TYPE;

using namespace odbc;
const std::unordered_map< std::string, int >& generateOdbcTypes() {
static std::unordered_map< std::string, int > _the_map;
INSTANTIATE_SQL_DATA_TYPE(CHAR);
INSTANTIATE_SQL_DATA_TYPE(VARCHAR)
INSTANTIATE_SQL_DATA_TYPE(LONGVARCHAR);

INSTANTIATE_SQL_DATA_TYPE(WCHAR);
INSTANTIATE_SQL_DATA_TYPE(WVARCHAR);
INSTANTIATE_SQL_DATA_TYPE(WLONGVARCHAR);

INSTANTIATE_SQL_DATA_TYPE(DECIMAL);
INSTANTIATE_SQL_DATA_TYPE(NUMERIC);
INSTANTIATE_SQL_DATA_TYPE(SMALLINT);
INSTANTIATE_SQL_DATA_TYPE(INTEGER);
INSTANTIATE_SQL_DATA_TYPE(REAL);
INSTANTIATE_SQL_DATA_TYPE(FLOAT);
INSTANTIATE_SQL_DATA_TYPE(DOUBLE);

INSTANTIATE_SQL_DATA_TYPE(BIT);
INSTANTIATE_SQL_DATA_TYPE(TINYINT);
INSTANTIATE_SQL_DATA_TYPE(BIGINT);

INSTANTIATE_SQL_DATA_TYPE(BINARY);
INSTANTIATE_SQL_DATA_TYPE(VARBINARY);
INSTANTIATE_SQL_DATA_TYPE(LONGVARBINARY);

INSTANTIATE_SQL_DATA_TYPE(TYPE_DATE);
INSTANTIATE_SQL_DATA_TYPE(TYPE_TIME);
INSTANTIATE_SQL_DATA_TYPE(TYPE_TIMESTAMP);
return _the_map;
}

//' Access the definition of the named SQL Data Type.
//'
//' @description Helper method returning the integer of the
//' [SQL Data Type](https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/sql-data-types?view=sql-server-ver16)
//' named in the argument.
//'
//' @seealso <https://github.com/microsoft/ODBC-Specification/blob/master/Windows/inc/sql.h>
//' @rdname odbcDataType.
//' @examples
//' \dontrun{
//' library(odbc)
//' ODBC_TYPE("LONGVARCHAR")
//' }
//' @export
// [[Rcpp::export]]
int ODBC_TYPE(const std::string& type) {
static const std::unordered_map< std::string, int >& map =
generateOdbcTypes();
auto itr = map.find(type);
if (itr != map.end())
{
return (int)itr->second;
}
utils::raise_error("Could not map input string to ODBC type");
return 0;
}
Loading