Skip to content

Split conversion functions into own module #15

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

Merged
merged 22 commits into from
Aug 14, 2018
Merged

Split conversion functions into own module #15

merged 22 commits into from
Aug 14, 2018

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Aug 10, 2018

The query.c module contained all the data conversion functions (SQL --> C, for columns --> app, as well as C --> SQL, for app --> parameters). It thus became quite large and hard to navigate.

This PR splits the conversion functions into own module, convert.[ch] and adds some small changes needed for that.

bpintea added 13 commits July 29, 2018 17:27
The setup API is meant to assist with installation of the driver.
stderr redirect will always work
- prompt_user_config() will pop up a dialog-box for connection config;
- prompt_user_overwrite() will confirm with the user that a config is to
be overwritten.
builds/.gitignore can get cleaned away, move its content into top
.gitignore
- add a DSN to start from;
- remove small code duplication in SQLDriverConnect;
- change config priorities in ConfigDSN(), as follows (most relevant
first):
  . 00-list received as parameter;
  . registry values;
  . defaults.
The values from the registry are read into a static buffer, indexed for
each entry into the esodbc_dsn_attrs_st structure.
Make sure that the indexing is done only for those values in the
Registry that have a correspondence in the structure.

Also, use SQLWCHAR explicitely instead of the TCHAR (should be same
values for Unicode compilation, but might generate issues later.)
Moved from all-static/all-dynamic to all-static, to simplify code
management.
- add a few more attributes to SQLGetInfo, some describing SQLGetData
extentions
- WIP on SQLGetData: set-up the temporary binding-unbinding on the
requested column.

Since SQLGetData requires no state maintenance except position in the
source data, the binding is done with a static ARD; if the column index
is lower-or-equal to a constant (128), the ARD will also use a static
array of records; otherwise a new array is allocated-freed.
Also:
- replace boolean conversion function, delegate that to the long long
conversion function;
- unify functions transfering SQLWCHAR and SQLCHAR strings
to application to minimize code duplication;
- build.bat : stop on error and propagate it to script exit;
- CMakeLists.txt: add flag for parallel building.
move around the 0-terminator accounting, for better code clarity
- moved respective functions to convert.[ch]
@bpintea bpintea mentioned this pull request Aug 10, 2018
Closed
@bpintea bpintea requested review from droberts195 and removed request for droberts195 August 10, 2018 18:47
a static array of 350K proves too large for the default 1MB stack, so
allocate it on heap
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

This seems to also contain all the changes from #14. It would be much easier to review if it could be rebased on master after #14 has been merged.

@bpintea
Copy link
Collaborator Author

bpintea commented Aug 13, 2018

Oh, right, sorry for that (last thing done on a Friday).
I've merged #14, but since that branch was forked off the 'setup_api' branch in #13, the merge covered those changes as well. If that's OK, I would close then #13 (since it got covered by #14)?
I would then also rebase 'convert_split' to remove the already merged changes.

- moved respective functions to convert.[ch]
a static array of 350K proves too large for the default 1MB stack, so
allocate it on heap
@bpintea
Copy link
Collaborator Author

bpintea commented Aug 13, 2018

This PR will now still show the full set of changes, but the diff between the elastic/master and bpintea/convert_split should be available now (
https://github.com/elastic/elasticsearch-sql-odbc/compare/master...bpintea:feature/convert_split?expand=1 ). Sorry again for the complication.

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@bpintea bpintea merged commit 3842cbf into elastic:master Aug 14, 2018
@bpintea bpintea deleted the feature/convert_split branch August 14, 2018 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants