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.

Sign up for GitHub

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

Document SQL dialect guidance #13706

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+54 −0
Open

Document SQL dialect guidance #13706

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6bb2aa8
Documentat SQL dialect followed
alamb Dec 9, 2024
5af035c
prettier
alamb Dec 9, 2024
49edb03
Apply suggestions from code review
comphead Dec 9, 2024
b674a42
try and clarify the postgres behavior refers to frontend
alamb Dec 10, 2024
8fd7f44
clarify how to change behavior
alamb Dec 10, 2024
e9f5da1
Update docs/source/user-guide/sql/dialect.md
alamb Dec 11, 2024
c6a83f3
Apply suggestions from code review
alamb Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/source/user-guide/sql/dialect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!---
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->

# SQL Dialect
Copy link
Member

Choose a reason for hiding this comment

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

it would be good to say this section pertains the SQL frontend:

  • sql parser and sql-to-rel
  • dataframe API
  • analyzer and the type coercions done at this stage
  • function semantics of functions bundled with datafusion

see also #13704 (comment)

it's also worth noting that we are not going to align with PostgreSQL's dialect fully

  • eg our type system is different (inherited from arrow)
  • we provide extended syntax for certain operations (like CREATE TABLE)
  • (...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworded and expanded in b674a42


By default, Apache DataFusion follows the [PostgreSQL SQL dialect].
Copy link
Member

Choose a reason for hiding this comment

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

When reading something that includes "by default", a reader may have natural question "how do i change this?"

Should we have a tip how to amend the behavior if the default doesn't please the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea -- I tried to do so in 8fd7f44

For Array/List functions and semantics, it follows the [DuckDB SQL dialect].

[duckdb sql dialect]: https://duckdb.org/docs/sql/functions/array
[postgresql sql dialect]: https://www.postgresql.org/docs/current/sql.html
Copy link
Member

Choose a reason for hiding this comment

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

Is this dualism good for users?

i think the market formed consensus to regard PostgreSQL-compatible as a good thing. Why do we water this down to say PostgreSQL-compatible, except where not? It's going to be confusing and incoherent

for example these expressions should be equivalent

  1. 2 IN (1, NULL, 3)
  2. 2 in (SELECT * FROM (VALUES (1), (NULL), (3)))
  3. is 2 in [1, NULL, 3] array

whereas we say that first two are governed by "be like PostgreSQL" goal
and the last is governed by "be like DuckDB" goal

Why would users appreciate that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how good this is for users -- I think this choice (to deviate from postgres and instead follow DuckDB) was driven by the desires of the original implementers. Maybe @jayzhan211 remembers more of the rationale

Copy link
Contributor

Choose a reason for hiding this comment

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

Nested functions are not well-supported in Postgres, so we choose DuckDB and other OLAP systems for these kind of function. We don’t enforce strict adherence to any particular system for function implementation. Instead, our goal is to establish a default behavior in the DataFusion core while making it easy to customize functions as needed. Given that Postgres and DuckDB are enough for us to decide what the function should be like so we ends up these 2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of focusing on which system we should be compatible with, our goal should be to make it easy to customize for different systems. For example, Spark and Postgres handle nulls differently, as does Datafusion. However, we currently lack a straightforward way to adjust null handling. Users are forced to copy entire functions and maintain them independently, which isn't ideal.

Copy link
Contributor

@jayzhan211 jayzhan211 Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

#6855 is the original discussion about switching to other OLAP system for nested function

Copy link
Contributor

Choose a reason for hiding this comment

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

But we need to admit the fact that we cannot just say "we follow dialect/database X, but for subcomponent we follow dialect/database Y", as this leads to inconsistencies.

I think the "inconsistency" mentioned here is what we want, learn the lesson from existing systems and choose the "better" one among them. The downside of "inconsistency" can be minimize if we document the rationale of the decision and the difference from other systems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I am trying to do with this PR is to provide practical guidance about what to do when we we hit a question like in #13683 (comment) (what should the array_has behavior be in the presence of nulls?)

I think it we could just say "do whatever DuckDB does in this case" that would be much more efficent and the PR would not get stuck

I realize there may not be complete alignment on how DataFusion's SQL should behave and I am in full support of making it as configurable as possible

I think there is a tension between

  1. Following an existing non ideal dialect (all existing dialects are non ideal) - which is predictable, and easier (we don't have to figure out semantics)
  2. Creating a new dialect that we feel is better - but now users have to learn that dialect and we have to define exactly what it means

Copy link
Member

Choose a reason for hiding this comment

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

Creating a new dialect that we feel is better - but now users have to learn that dialect and we have to define exactly what it means

I also would prefer to avoid doing that.
But in fact we're doing exactly that. It's a stretch to pretend we're not shipping "DataFusion's dialect of SQL".

I think it we could just say "do whatever DuckDB does in this case" that would be much more efficent and the PR would not get stuck

Yes, that's efficient.
But same efficiency you get from "do whatever PostgreSQL does" (efficiency in terms of answering on-PR design questions)
And same efficiency you get from "do whatever Trino does"

We're trading answer efficiency vs consistency and predictability of our of the box semantics, with both sides of the trade being desirable properties.

It's likely that maximum consistency you get from "do whatever SQL spec says", but obviously this isn't very efficient (most engineers can't even read the spec).

Following an existing non ideal dialect (all existing dialects are non ideal) - which is predictable, and easier (we don't have to figure out semantics)

That would be great, especially if this a reasonable dialect (some are, some are not) and is one dialect, not some form of a synergy between two different dialects that disagree with each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can change this section to be titles "Aspirational SQL dialect" or something.

It's a stretch to pretend we're not shipping "DataFusion's dialect of SQL".

I fully admit that what is supported by DataFusion is not exactly the same as any existing dialect. Maybe it is wishful thinking that we are "close enough" to postgres and that defining the goal (even if we never reach it) is valuable 🤔

Copy link
Member

Choose a reason for hiding this comment

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

defining the goal (even if we never reach it) is valuable 🤔

Agreed!

We should have a goal, and we should have a litmus test for determining what gets in and with what semantics


## Rationale

SQL Engines have a choice to either use an existing SQL dialect or define their
own. Using an existing dialect may not fit perfectly as it is hard to match
semantics exactly (need bug-for-bug compatibility), and is likely not what all
Copy link
Contributor

Choose a reason for hiding this comment

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

More reasons for using pogstgres specifically: it is

  • rather extensive, in contrast to e.g. sqlite
  • rather well aligned w/ the SQL standard (at least that's my personal impression, after having faced MySQL)

Copy link
Member

Choose a reason for hiding this comment

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

rather well aligned w/ the SQL standard (at least that's my personal impression, after having faced MySQL)

mostly true (but i know of some deviations)
if we wanted something "executable but also aligned with SQL std", I'd recommend Trino

i kind of assumed PostgreSQL ship has sailed and we're just retro-documenting. But if the ball (choice) is still in play, my vote goes to Trino as a good reference implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i kind of assumed PostgreSQL ship has sailed and we're just retro-documenting.

That was my assumption too, but as others like @jayzhan211 (and yourself) have pointed out, I don't think DataFusion is (or can be) 100% consistent with this point

users want. However, it avoids the (very significant) effort of defining
semantics as well as documenting and teaching users about them.

As Apache DataFusion is highly customizable, systems built on DataFusion can and do
update functions and SQL syntax to model other systems, such as Apache Spark or
Copy link
Member

Choose a reason for hiding this comment

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

and type rules

MySQL.
1 change: 1 addition & 0 deletions docs/source/user-guide/sql/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ SQL Reference
.. toctree::
:maxdepth: 2

dialect
data_types
select
subqueries
Expand Down
Loading