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

Data buffer fixes #24

Merged
merged 6 commits into from
Aug 3, 2016
Merged

Data buffer fixes #24

merged 6 commits into from
Aug 3, 2016

Conversation

jgoizueta
Copy link

Fixes #23

Deal with the case that the buffer for SQLGetData is too small,
and also with missing trailing zeros.
Some types (e.g. PostGIS geometry columns which are mapped to text)
may have huge sized. We now use type text for those large columns
and limit the buffer size (which is no problem since
GetData by parts was implemented)
* Non-supported column types (including bit-strings) are now omitted
* bit(1) is interpreted as boolean
* Conversion to bytea (longvarbinary) is handled properly

The buf_used variable has been added to keep track of the buffer size
in case of binary data (non null-terminated) so it will be available
for future binary conversions.
@jgoizueta
Copy link
Author

@rafatower please review. I think review may be easier looking at each commit in isolation

* * With options BoolsAsChar=0 this allows
* preserving boolean columns from pSQL ODBC.
*/
appendStringInfo(sql_type, "boolean");

Choose a reason for hiding this comment

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

indentantion

please start using a linter or fomater of your choice.

Copy link
Author

Choose a reason for hiding this comment

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

I think that misalignment is due to how github handles tabs, but I sure do need a linter

@rafatower
Copy link

My understanding is that this fixes some issues found with pg driver, right?

What are the chances of breaking any of the working drivers with this patch? what can be done to mitigate that risk?

@jgoizueta
Copy link
Author

jgoizueta commented Aug 2, 2016

This deals with problems found with Hive and PG (but could affect other drivers too), namely:

  • The driver reported size for a column may be insufficient for all its values
  • A number for which the read buffer is too small will be truncated and we cannot read it in parts
  • The reported size of column can be huge for varying-size columns
  • PG booleans are mapped to ODBC bit(1) or char(5) depending on a parameter
  • ODBC LongVarBinary type is formatted as hexadecimal when read through a C string (so this, unlike the bit strings can be read easily into a bytea).

It has been tested with PG, MySQL and Hive. (well, binary data has not been tested with Hive, but it probably didn't work as it was)

@rafatower rafatower merged commit 3ea8a68 into master Aug 3, 2016
@rafatower rafatower deleted the 23-getdata branch August 3, 2016 15:19
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