Document SQL dialect guidance

[alamb]

Which issue does this PR close?

• Closes Document the SQL dialect DataFusion attempts to follow #13704

Rationale for this change

Determining what is the "correct behavior" comes frequently deciding how a function should behave and there is disagreement across implementations (e.g. postgres does something different than spark and/or duckdb)

This happened most recently here with @comphead and @jayzhan211 and @Kimahriman in #13683 (comment)

What changes are included in this PR?

1. Try and document what the current behavior is (please double check me here)

Are these changes tested?

By CI

Are there any user-facing changes?

New docs page.

In my mind this doesn't change any semantics or policy, it simply documents the current reality. However, I am not 100% everyone has the same understanding of the current reality 🤔

[Kimahriman]

Is it confusing when an array/list function exists in PostgreSQL but you don't follow it's semantics, even though you do for non array/list functions?

[alamb]

I think it is confusing for sure

[comphead]

its good to get this documented
Thanks @alamb

[alamb]

I plan to leave this open for several days to allow additional time for reviews / comments

[findepi]

and type rules

[findepi]

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?

[alamb]

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

[findepi]

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)
• (...)

[alamb]

Reworded and expanded in b674a42

[findepi]

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?

[alamb]

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

[jayzhan211]

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.

[jayzhan211]

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.

[jayzhan211]

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

[jayzhan211]

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.

[alamb]

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

[findepi]

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.

[alamb]

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 🤔

[findepi]

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

[alamb]

Thank you @findepi and @comphead for the reviews

[alamb]

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

[alamb]

Reworded and expanded in b674a42

[alamb]

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

[crepererum]

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)

[findepi]

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.

[alamb]

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

[findepi]

I agree we shouldn't follow PostgreSQL arrays as the guide. PostgreSQL arrays are not structured. You can declare dimensions of an array column, but that's advisory only. Each value can be array of different dimensions. That won't work well with Arrow type system - there is no arrow type that corresponds to PostgreSQL array type.

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.

Yes, we will address needs of downstream projects by providing necessary extension points. In the worst case, they can override "everything".
But DataFusion also has its frontend and it should strive to be consistent. Consistency is important both to end users and also to people building on top of datafusion.

[findepi]

Not quite? DuckDB array seems to be fixed size. Is this saying DF array follows DuckDB's list semantics?

[jayzhan211]

yes DuckDB's "list" not "array". We prefer to convert fixed-size lists into regular lists, so we don’t make a distinction between the two

[findepi]

is this going to be obvious to the reader of this document?

[findepi]

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.