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

ARROW-11165: [Rust][DataFusion] Document Postgres as standard SQL dialect #9127

Closed
wants to merge 2 commits into from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 7, 2021

PROPOSAL Document postgres as the target SQL / function dialect and rationale for this choice. I will also send an email to the dev mailing list soliciting feedback

There are several comments and more discussion on #9108

@github-actions
Copy link

github-actions bot commented Jan 7, 2021

@jorgecarleitao
Copy link
Member

I agree. Another advantage is that it focus development towards implementing that dialect. :)

@Dandandan
Copy link
Contributor

Makes sense to me, seems a strategy more are following lately? I would suggest to strive for compatibility with the PostgreSQL dialect, but also allow for some extensions such as functions that are not in PostgreSQL but still a valuable addition to DataFusion.

@seddonm1
Copy link
Contributor

seddonm1 commented Jan 7, 2021

@alamb thanks for organising this!

I agree. Another advantage is that it focus development towards implementing that dialect. :)

Yes, I was hoping once we reach a decision we can do a comparison between what we have and what Postgres has then just work through the list.

@alamb
Copy link
Contributor Author

alamb commented Jan 8, 2021

Yes, I was hoping once we reach a decision we can do a comparison between what we have and what Postgres has then just work through the list.

Sounds like a good plan @seddonm1 -- given that postgres has more than 20 years of official releases I suspect the list will be long and distinguished :)

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM

@alamb
Copy link
Contributor Author

alamb commented Jan 9, 2021

I plan to merge this on Monday Jan 11 2021 unless I hear any objections


DataFusion implements a subset of the [PostgreSQL SQL dialect](https://www.postgresql.org/docs/current/functions.html) where possible. We explicitly choose a single dialect to maximize interoperability with other tools and allow reuse of the PostgreSQL documents and tutorials as much as possible.

Currently, only a subset of the PosgreSQL dialect is implemented, and we will document any deviations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a list or table somewhere that shows what we currently support?

@seddonm1
Copy link
Contributor

seddonm1 commented Jan 10, 2021 via email

rust/datafusion/README.md Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #9127 (0b60268) into master (3622a2e) will decrease coverage by 0.79%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9127      +/-   ##
==========================================
- Coverage   82.59%   81.80%   -0.80%     
==========================================
  Files         204      214      +10     
  Lines       50526    51383     +857     
==========================================
+ Hits        41732    42033     +301     
- Misses       8794     9350     +556     
Impacted Files Coverage Δ
rust/datafusion/src/datasource/csv.rs 65.00% <0.00%> (-16.25%) ⬇️
rust/datafusion/src/datasource/parquet.rs 94.77% <0.00%> (-1.44%) ⬇️
rust/datafusion/src/physical_plan/expressions.rs 83.77% <0.00%> (-0.71%) ⬇️
rust/datafusion/src/sql/utils.rs 53.92% <0.00%> (-0.68%) ⬇️
rust/datafusion/src/optimizer/utils.rs 58.18% <0.00%> (-0.54%) ⬇️
rust/arrow/src/ipc/writer.rs 83.21% <0.00%> (-0.05%) ⬇️
rust/arrow-flight/src/utils.rs 0.00% <0.00%> (ø)
rust/arrow/src/ipc/gen/Schema.rs 39.71% <0.00%> (ø)
rust/arrow/src/array/equal/boolean.rs 100.00% <0.00%> (ø)
rust/arrow/src/ipc/gen/SparseTensor.rs 0.00% <0.00%> (ø)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3622a2e...0b60268. Read the comment docs.

@alamb
Copy link
Contributor Author

alamb commented Jan 11, 2021

Given no other comments or opinions to the contrary I am merging this in

@alamb alamb closed this in 3be7592 Jan 11, 2021
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
…lect

PROPOSAL Document postgres as the target SQL / function dialect and rationale for this choice. I will also send an email to the dev mailing list soliciting feedback

There are several comments and more discussion on apache#9108

Closes apache#9127 from alamb/alamb/ARROW-11165-dialect

Authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants