Skip to content

Conversation

@Mpdreamz
Copy link
Member

No description provided.

@Mpdreamz Mpdreamz mentioned this pull request Sep 24, 2018
89 tasks
Copy link
Contributor

@russcam russcam left a comment

Choose a reason for hiding this comment

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

LGTM 👍 there are folks already looking forward to this!

I left a few small comments

fields.And(field);
reader.Read(); // "format";
reader.Read(); // "format value";
reader.Read(); // "}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a test for this?

/// and can take any values as documented on Joda time's DateTimeZone enum.
/// </summary>
[JsonProperty("time_zone")]
string TimeZone { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

We should look at standardising TimeZone and Timezone in 7.x


namespace Nest
{
public class SqlRow : ReadOnlyCollection<SqlValue> {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: formatting of { not on separate line

@russcam russcam merged commit a59a00a into 6.4 Oct 16, 2018
@russcam russcam deleted the feature/6.4-sql branch October 18, 2018 09:45
russcam pushed a commit that referenced this pull request Oct 26, 2018
* Add QuerySql API methods and tests
* Add ClearSqlCursor API methods and tests
* Add TranslateSql API methods and tests

(cherry picked from commit a59a00a)
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.

3 participants