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

Support smallint, tinyint, date type of Cassandra #15147

Merged
merged 1 commit into from
Sep 16, 2020

Conversation

SandishKumarHN
Copy link
Contributor

@SandishKumarHN SandishKumarHN commented Sep 9, 2020

Support smallint, tinyint, date type of Cassandra

== RELEASE NOTES ==

Cassandra Change
* Add `SMALLINT`, `TINYINT`, and `DATE` type support to Cassandra connector.

@SandishKumarHN SandishKumarHN force-pushed the CassandraSupportSmallInt branch from 8d3ebef to f069ba4 Compare September 9, 2020 19:17
@dborkar
Copy link

dborkar commented Sep 9, 2020

@highker or @wenleix , Can you please help review? Thx.

@@ -127,6 +134,8 @@ public static CassandraType getCassandraType(DataType.Name name)
return COUNTER;
case CUSTOM:
return CUSTOM;
case DATE:
Copy link
Member

Choose a reason for hiding this comment

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

better to document these types support in cassandra.rst

@SandishKumarHN SandishKumarHN force-pushed the CassandraSupportSmallInt branch 2 times, most recently from 30e6cf7 to e5d59c4 Compare September 16, 2020 13:18
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 16, 2020

CLA Check
The committers are authorized under a signed CLA.

  • ✅ Yuya Ebihara (e5d59c47ccfecde9e87b6962f9ef4d5319fd378b)
  • ✅ SandishKumarHN (f2c8b9eed07e76aa3aea51f01acfd8283aaa4979)

@fgwang7w
Copy link
Member

LGTM. I'd defer to @highker if there's any more comments.
@SandishKumarHN There is an integration test failure, please double check if you need to do rebasing. Thanks!

@SandishKumarHN
Copy link
Contributor Author

LGTM. I'd defer to @highker if there's any more comments.
@SandishKumarHN There is an integration test failure, please double check if you need to do rebasing. Thanks!

@fgwang7w the integration test failure is not related to PR.

Copy link
Contributor

@highker highker left a comment

Choose a reason for hiding this comment

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

Could you sign CLA?

Comment on lines 88 to 91
this.toCassandraDate = value -> DATE_FORMATTER.print(TimeUnit.DAYS.toMillis(value));
}
else {
this.toCassandraDate = value -> LocalDate.fromDaysSinceEpoch(toIntExact(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

@highker highker self-assigned this Sep 16, 2020
@highker
Copy link
Contributor

highker commented Sep 16, 2020

and squash two commits into one?

@SandishKumarHN SandishKumarHN force-pushed the CassandraSupportSmallInt branch from f2c8b9e to 03becae Compare September 16, 2020 18:25
@SandishKumarHN
Copy link
Contributor Author

and squash two commits into one?

@highker done, and CI failure is not related to the PR.

@highker highker merged commit 06d5f49 into prestodb:master Sep 16, 2020
@ajaygeorge
Copy link
Contributor

Unfortunately this has resurfaced the joda libraries which were taken out in #14046 :(

v-jizhang added a commit to v-jizhang/presto that referenced this pull request May 24, 2021
Cherry-pick of trinodb/trino#141. This is
also a fix for prestodb#15147 which
tried to backport Trino prestodb#141 but that backporting was incomplete
and caused the Cassandra tests to fail.

Resolves: prestodb#15749

Co-authored-by: Yuya Ebihara <ebyhry@gmail.com>
highker pushed a commit that referenced this pull request May 25, 2021
Cherry-pick of trinodb/trino#141. This is
also a fix for #15147 which
tried to backport Trino #141 but that backporting was incomplete
and caused the Cassandra tests to fail.

Resolves: #15749

Co-authored-by: Yuya Ebihara <ebyhry@gmail.com>
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.

6 participants