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

DATE type initial support #100

Merged
merged 6 commits into from
Jan 28, 2019
Merged

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Jan 28, 2019

This PR adds initial support for the new DATE ES/SQL (runtime-only) data type (elastic/elasticsearch#37693): the date type will be accepted by the driver and passed onto the application, but no data conversions are possible yet (should applications request them).

The PR also changes the connection testing: the "simple" query has been replaced by data type loading. This improves the testing by setting up the connection just like before "normal" operation would. (Specifically, data types misalignment would no longer be possible, when connection testing would work, but application connecting would later fail.)

A couple of fixes are also addressed:

  • incorrect allocation for server version string (the space reservation for the null terminator had been done incorrectly); and
  • wrong data type structure member had been debug-logged previously.

- also, extend init_diagnostic() to reset all diagnostic fields.
- replace the 'SELECT 0' connection testing with a better test: ES/SQL
data types loading. This also better fits with do_connect() function
fully preparing the connection for usage.
If using a running staged instance skip the (in this case) unnecessary
version parameter validation.
- fix allocation size of server version string;
- fix column parameter logging.
- the type is now understood by the driver
  (no data conversion supported yet)
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, but I think the PR should be labelled with either fix or enhancement, not both - it should only go into one section in the release notes so decide whether you think it fits best as a bug fix or enhancement in the release notes.

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 removed the fix label Jan 28, 2019
@bpintea bpintea merged commit 90c79bf into elastic:master Jan 28, 2019
@bpintea bpintea deleted the feature/date_type branch January 28, 2019 12:51
bpintea added a commit that referenced this pull request Jan 29, 2019
* add a diagnostics resetting macro

- also, extend init_diagnostic() to reset all diagnostic fields.

* replace simple conn test with SQL data type load

- replace the 'SELECT 0' connection testing with a better test: ES/SQL
data types loading. This also better fits with do_connect() function
fully preparing the connection for usage.

* fix: don't require ES version if using staged inst

If using a running staged instance skip the (in this case) unnecessary
version parameter validation.

* fix: correct alloc size and logged struct field

- fix allocation size of server version string;
- fix column parameter logging.

* add DATE data type initial support

- the type is now understood by the driver
  (no data conversion supported yet)

* add DATE initial support into u-testing framework

(cherry picked from commit 90c79bf)
@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 v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants