Skip to content

Configurable floats format on conversions #130

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 4 commits into from
Mar 20, 2019

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Mar 19, 2019

This PR adds a new config paramter to the connection string that instructs the driver to convert the floats into the default, scientific or auto-mode notation; auto meaning scientific if the value is small or
large "enough".

It also uses scale received from Elasticsearch for the respective type.
A defect was corrected here, the application now no longer being able to specify the scale for conversion (this is source specific).

Also, on the consistency checks the driver does on descriptors, the precision and scale are now also those read from Elasticsearch (rather than local values).

bpintea added 3 commits March 19, 2019 21:40
- in case password generation fails, log the messages generated by the
script.
- also, in case another Elasticsearch instance is running, fail with the
correct error message.
- in case there's an unexpected data type received from Elasticsearch
(ex.: float value for DATE) make sure the application receives a
specific diagnostic.
This commit adds a new config paramter to the connection string that
instructs the driver to convert the floats into the default, scientific
or auto-mode notation; auto meaning scientific if the value is small or
large "enough".

It also uses scale received from Elasticsearch for the respective type.

Also, on the consistency checks the driver does on descriptors, the
precision and scale are now also those read from Elasticsearch (rather
than local values).
driver/convert.c Outdated
static SQLRETURN double_to_str(esodbc_rec_st *arec, esodbc_rec_st *irec,
double dbl, void *data_ptr, SQLLEN *octet_len_ptr, BOOL wide)
{
# define DBL_BASE10_MAX_LEN /*-0.*/3 + DBL_MANT_DIG - DBL_MIN_EXP
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe DBL_MANT_DIG and DBL_MIN_EXP refer to the base 2 representation, not base 10, so they're 53 and -1021 respectively. The base 10 equivalents are 15 and -308. In other words the number of characters to print the smallest double in fixed point notation would be 3 + 15 - -308, not 3 + 53 - -1021. I think the correct constants in C99 are DBL_DIG and DBL_MIN_10_EXP. But if Microsoft's headers don't define these or if you want to be allow a bit more in case there's some edge case where more digits are printed then maybe just define a constant to slightly higher like 336 (= 21 * 16) and use that for the buffer size including the \0 terminator?

Copy link
Collaborator Author

@bpintea bpintea Mar 20, 2019

Choose a reason for hiding this comment

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

Thanks for spotting that, fixed now.
The defs are available (https://docs.microsoft.com/en-us/cpp/cpp/floating-limits).
I've stuck with the "exact" calculation, since the buffer should anyways be way over-provisioned for the precision (=ODBC's scale) these numbers will be printed with. (Currently the limit is 19, but even that is probably too high -- pending further clarifications.)

- addressing PR comments.
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 d61f3cf into elastic:master Mar 20, 2019
@bpintea bpintea deleted the feature/floats_to_str branch March 20, 2019 19:30
bpintea added a commit that referenced this pull request Mar 26, 2019
* integ. test.: more verbosity on pwd gen. failure

- in case password generation fails, log the messages generated by the
script.
- also, in case another Elasticsearch instance is running, fail with the
correct error message.

* fix: return error upstream in case of API errors

- in case there's an unexpected data type received from Elasticsearch
(ex.: float value for DATE) make sure the application receives a
specific diagnostic.

* add configurable mode for floats conversion

This commit adds a new config paramter to the connection string that
instructs the driver to convert the floats into the default, scientific
or auto-mode notation; auto meaning scientific if the value is small or
large "enough".

It also uses scale received from Elasticsearch for the respective type.

Also, on the consistency checks the driver does on descriptors, the
precision and scale are now also those read from Elasticsearch (rather
than local values).

* fix: use radix 10 for dbl buff limits

- addressing PR comments.

(cherry picked from commit d61f3cf)
@bpintea bpintea added >feature Applicable to PRs adding new functionality and removed >enhancement labels May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Applicable to PRs adding new functionality v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants