Skip to content

Conversation

@bpintea
Copy link
Collaborator

@bpintea bpintea commented Aug 15, 2019

This PR bundles a few minor fixes:

  1. A check on conversion failure was incorrectly taking into account the result of a previous column conversion on the same row. The defect was without practical implications, since a second, impossible condition would have had to match to lead to an incorrect failure. However the incorrect first check lead to a failing assertion.

  2. The IRD descriptor was not re-init'd on statement handle closing. This defect is fixed just for good house-keeping, since the descriptor would be re-init'd on attaching a new result set to it (which happens regularly when the result for a query is being paged / using a cursor). Along with this, some code duplication has been removed.

  3. A few logging statements had the wrong format specifiers, mostly expecting signed, instead of the correct unsigned types of the integer variants.

The timestamp/time to string truncation needs to fail only if current
status code (for current column) indicates that truncation occured; so
status code has been added to the branching evaluation.

Previously, only the current diagnostic code was used in the decision
(and checked if it indicated truncation), which was incorrect since this
could have been set by the conversion of a previous column in same row.
On statment closing the IRD descriptor's headers should be
reinitialized.
The fix is for good house keeping, the defect didn't have a
practical impact, since the IRD is cleared also before adding a new
result set; furthermore, a new result set added to an already-used IRD
would only occur if a statement handle is re-prepared, which happens
very rarely (generally a client app would allocate a new handle for a
new query).

The commit also switches a few catalog functions to using
SQLExecDirect(), instead of dupplicating its code.
Correct some format specifiers for logging, mostly s/zd/zu for size_t
types.
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

@bpintea bpintea merged commit c7aa95b into elastic:master Aug 16, 2019
@bpintea bpintea deleted the fix/correct_diag_on_truncation branch August 16, 2019 11:09
bpintea added a commit that referenced this pull request Aug 16, 2019
…losing. Logging format descriptors (#170)

* fix: only fail time*/str conversion on truncation

The timestamp/time to string truncation needs to fail only if current
status code (for current column) indicates that truncation occured; so
status code has been added to the branching evaluation.

Previously, only the current diagnostic code was used in the decision
(and checked if it indicated truncation), which was incorrect since this
could have been set by the conversion of a previous column in same row.

* fix: re-init IRD headers on statement close

On statment closing the IRD descriptor's headers should be
reinitialized.
The fix is for good house keeping, the defect didn't have a
practical impact, since the IRD is cleared also before adding a new
result set; furthermore, a new result set added to an already-used IRD
would only occur if a statement handle is re-prepared, which happens
very rarely (generally a client app would allocate a new handle for a
new query).

The commit also switches a few catalog functions to using
SQLExecDirect(), instead of dupplicating its code.

* fix: correct logging format specifiers

Correct some format specifiers for logging, mostly s/zd/zu for size_t
types.

(resolved conflict around '/* ESODBC_CURL_CLOSE */' chunk)

(cherry picked from commit c7aa95b)
bpintea added a commit that referenced this pull request Aug 28, 2019
…losing. Logging format descriptors (#170)

* fix: only fail time*/str conversion on truncation

The timestamp/time to string truncation needs to fail only if current
status code (for current column) indicates that truncation occured; so
status code has been added to the branching evaluation.

Previously, only the current diagnostic code was used in the decision
(and checked if it indicated truncation), which was incorrect since this
could have been set by the conversion of a previous column in same row.

* fix: re-init IRD headers on statement close

On statment closing the IRD descriptor's headers should be
reinitialized.
The fix is for good house keeping, the defect didn't have a
practical impact, since the IRD is cleared also before adding a new
result set; furthermore, a new result set added to an already-used IRD
would only occur if a statement handle is re-prepared, which happens
very rarely (generally a client app would allocate a new handle for a
new query).

The commit also switches a few catalog functions to using
SQLExecDirect(), instead of dupplicating its code.

* fix: correct logging format specifiers

Correct some format specifiers for logging, mostly s/zd/zu for size_t
types.

(cherry picked from commit c7aa95b)
bpintea added a commit that referenced this pull request Dec 4, 2019
…losing. Logging format descriptors (#170)

* fix: only fail time*/str conversion on truncation

The timestamp/time to string truncation needs to fail only if current
status code (for current column) indicates that truncation occured; so
status code has been added to the branching evaluation.

Previously, only the current diagnostic code was used in the decision
(and checked if it indicated truncation), which was incorrect since this
could have been set by the conversion of a previous column in same row.

* fix: re-init IRD headers on statement close

On statment closing the IRD descriptor's headers should be
reinitialized.
The fix is for good house keeping, the defect didn't have a
practical impact, since the IRD is cleared also before adding a new
result set; furthermore, a new result set added to an already-used IRD
would only occur if a statement handle is re-prepared, which happens
very rarely (generally a client app would allocate a new handle for a
new query).

The commit also switches a few catalog functions to using
SQLExecDirect(), instead of dupplicating its code.

* fix: correct logging format specifiers

Correct some format specifiers for logging, mostly s/zd/zu for size_t
types.

Cherry-pick merging:
- truncation failure check not applicable.
@bpintea bpintea added the v6.8.6 label Dec 4, 2019
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