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

[CALCITE-6257] StarRocks dialect implementation #3682

Closed
wants to merge 1 commit into from

Conversation

YiwenWu
Copy link
Contributor

@YiwenWu YiwenWu commented Feb 11, 2024

Implementing StarRocks dialect in Calcite
https://issues.apache.org/jira/browse/CALCITE-6257

/**
* A <code>SqlDialect</code> implementation for the StarRocks database.
*/
public class StarRocksSqlDialect extends SqlDialect {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know anything about the StarRocks dialect, so my review will assume that these choices are correct for that dialect. I am reviewing only from the Calcite side.

Copy link
Member

@caicancai caicancai Feb 15, 2024

Choose a reason for hiding this comment

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

@mihaibudiu Hello. As far as I know, the sql dialect of starrocks is compatible with mysql, but starrocks is an olap database that supports more data types and some special sql functions.

@@ -4506,13 +4600,16 @@ private void checkLiteral2(String expression, String expected) {
String expectedPresto = "SELECT DATE_TRUNC('MINUTE', \"hire_date\")\n"
+ "FROM \"foodmart\".\"employee\"";
String expectedFirebolt = expectedPostgresql;
String expectedStarRocks = "SELECT DATE_FORMAT(`hire_date`, '%Y-%m-%d %H:%i:00')\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong, DATE_FORMAT returns a string, while DATE_TRUNC returns a date.

Copy link
Contributor Author

@YiwenWu YiwenWu Feb 18, 2024

Choose a reason for hiding this comment

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

Indeed, this test case directly uses MysqlSqlDialect#unparseFloor, MySQL/StarRocks Function DATE_FORMAT returns a string. date_format docs

StarRocks also support function date_trunc. Refer to the processing of PostgresqlSqlDialect and replaced the FLOOR function with DATE_TRUNC


@Override public @Nullable SqlNode getCastSpec(RelDataType type) {
switch (type.getSqlTypeName()) {
case TIMESTAMP:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused what's going on here.
If the Calcite TIMESTAMP type is called "DATETIME" in StarRocks, I am not sure whether this is the only case that needs changing. Can you write some tests that use TIMESTAMP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, Similar to MySQL, the Calcite TIMESTAMP type is called "DATETIME" in StarRocks. This processing refers to MysqlSqlDialect#getCastSpec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional TIMESTAMP test:

  • RelToSqlConverterTest#testCastToTimestamp
  • RelToSqlConverterTest#testUnparseSqlIntervalQualifier

@YiwenWu YiwenWu force-pushed the starrocks_dialect branch 3 times, most recently from 295dd55 to 1f000ec Compare February 19, 2024 03:14
Copy link

sonarcloud bot commented Feb 19, 2024

@macroguo-ghy
Copy link
Contributor

Please solve the conflicts first.

from starrocks doc, Starrocks say 'Compatible with MySQL', so maybe StarrocksSqlDialect can extend from MysqlSqldialect?

@YiwenWu
Copy link
Contributor Author

YiwenWu commented Feb 22, 2024

Please solve the conflicts first.

from starrocks doc, Starrocks say 'Compatible with MySQL', so maybe StarrocksSqlDialect can extend from MysqlSqldialect?

I'll try to handle it like this.

@YiwenWu YiwenWu closed this Feb 22, 2024
@YiwenWu YiwenWu deleted the starrocks_dialect branch February 22, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants