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

CBOR support for result sets #172

Merged
merged 3 commits into from
Aug 16, 2019

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Aug 15, 2019

This PR extends the CBOR support with the ability to read the
CBOR-encapsulated result sets.

The PR also makes fetching data more efficient with the SQLGetData()
(the alternative to the generally more efficient SQLBindCol()). The
implementation still uses punctual column binding and unbinding, but
SQLFetch() will now cache the source JSON/CBOR object into IRD's records
and will no longer walk the entire list of instantied ARD records all
the way to the ad-hoc bound column.

Counting the total number of rows returned for a query has been changed
to cope with ES's use of indefinite-size arrays, to avoid iterating
twice over the rows in a page.

- this ends up to simply the spec file a bit and removes some
unnecessary code in the library.
This commit extends the CBOR support with the ability to read the
CBOR-encapsulated result sets.

The commit also makes fetching data more efficient with the SQLGetData()
(the alternative to the generally more efficient SQLBindCol()). The
implementation still uses punctual column binding and unbinding, but
SQLFetch() will now cache the source JSON/CBOR object into IRD's records
and will no longer walk the entire list of instantied ARD records all
the way to the ad-hoc bound column.

Counting the total number of rows returned for a query has been changed
to cope with ES's use of indefinite-size arrays, to avoid iterating
twice over the rows in a page.
@bpintea bpintea added >feature Applicable to PRs adding new functionality v8.0.0-alpha1 v7.4.0 labels Aug 15, 2019
@edsavage edsavage self-assigned this Aug 15, 2019
Copy link
Collaborator

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM

driver/queries.c Outdated
}
if (msg) {
res = ascii_c2w((SQLCHAR *)msg, wbuff, SQL_MAX_MESSAGE_LENGTH - 1);
wmsg = 0 < res ? wbuff : NULL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic could be slightly simplified if wmsg is initialised to NULL above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated.
I was avoiding the potentially superfluous assignment (in case msg != NULL and 0 < res), but that's a thing the compiler can easily optimize away and this is anyways executed on the error case, so simplification is welcome here.
thanks.

@bpintea bpintea mentioned this pull request Aug 16, 2019
5 tasks
- slight code simplification
@bpintea bpintea merged commit 4fc9197 into elastic:master Aug 16, 2019
@bpintea bpintea deleted the feat/pack_cbor_resultset branch August 16, 2019 12:26
bpintea added a commit that referenced this pull request Aug 28, 2019
* cbor: add source files individually to the project

- this ends up to simply the spec file a bit and removes some
unnecessary code in the library.

* add CBOR support for result sets

This commit extends the CBOR support with the ability to read the
CBOR-encapsulated result sets.

The commit also makes fetching data more efficient with the SQLGetData()
(the alternative to the generally more efficient SQLBindCol()). The
implementation still uses punctual column binding and unbinding, but
SQLFetch() will now cache the source JSON/CBOR object into IRD's records
and will no longer walk the entire list of instantied ARD records all
the way to the ad-hoc bound column.

Counting the total number of rows returned for a query has been changed
to cope with ES's use of indefinite-size arrays, to avoid iterating
twice over the rows in a page.

* addressing PR review comments

- slight code simplification

(cherry picked from commit 4fc9197)
@bpintea bpintea mentioned this pull request Sep 10, 2019
Closed
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.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants