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

[Discussion] SQL dialect #855

Open
Tracked by #722
Yury-Fridlyand opened this issue Sep 23, 2022 · 3 comments
Open
Tracked by #722

[Discussion] SQL dialect #855

Yury-Fridlyand opened this issue Sep 23, 2022 · 3 comments
Labels
design documentation Improvements or additions to documentation

Comments

@Yury-Fridlyand
Copy link
Collaborator

OpenSearch SQL extends ANSI SQL. I propose not to create a new dialect, but align with existing one.
Some features already follow MySQL syntax and implementation. MySQL was selected because it is one of the most popular.
The discussion in this thread should confirm (or not) following MySQL dialect. Any procs, cons, ideas, objectives.
To finalize this task we need to update documentation and readme files.

@Yury-Fridlyand Yury-Fridlyand added documentation Improvements or additions to documentation design labels Sep 23, 2022
@Yury-Fridlyand
Copy link
Collaborator Author

See also: #852

@MaxKsyunz
Copy link
Collaborator

MaxKsyunz commented Sep 23, 2022

Thank you @Yury-Fridlyand for starting this discussion -- it's important one to have and document.

In my view, SQL plugin should be consistent with other parts of OpenSearch and development ecosystem first and foremost.
In particular,

  1. Any query written in SQL and equivalent OpenSearch DSL should produce the same result.
  2. Implementation should make sense in the context of Java.

Violating either of these will be a source of bugs for us and create pitfalls for end-users and application developers.

#852 is a good example of this. To support TIME values > 24 hours we will have to add a custom data type instead of using java.sql.Time. It will force JDBC clients to special case such fields as well. That's all error-prone and expensive, what use case does TIME > 24 hours enable?

From another angle, OpenSearch SQL is already meaningfully incompatible[*] with MySQL. Given that, let's focus on expanding the feature set as easily as we can in a consistent manner.

[*] There's a valid OpenSearch SQL query that is invalid MySQL query and vice versa -- there's a valid MySQL query that is invalid OpenSearch SQL.

@Yury-Fridlyand
Copy link
Collaborator Author

We had a discussion outside of GH and confirmed that parser and constructor-like functions (TIME, DATE, DATETIME, TIMESTAMP) should throw an exception on invalid string input, instead of returning NULL. Even though we are trying to align with MySQL, this behavior contradicts MySQL.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants